RonaldJerez / vue-input-facade

A lightweight and dependency free input masking library created specific for Vue
https://ronaldjerez.github.io/vue-input-facade/latest/
182 stars 27 forks source link

fix: firefox issue where masked value can overflow #32

Closed tskimmett closed 3 years ago

tskimmett commented 3 years ago

Description

Firefox has a bug/inconsistency in their DOM event implementation which allows "bubbled" event listeners (useCapture: false) to be executed before "captured" event handlers (useCapture: true) when the listeners are placed on the same event target.

The effect of this inconsistency is that a standard vue event listener (@input="handleInput") can execute before the masking directive has a chance to process/capture the original input event, leading to potential inconsistencies where downstream event handlers receive a different value for the input than what is actually reflected by the DOM.

To avoid this problem, this change set updates the directive by adding the input event listener on the input element's parent, instead of the input element itself. Placing the listener on the parent ensures the correct execution order (capture listeners before bubble listeners).

Quote from this issue comment for more context and explanation:

The vue-input-facade directive adds a capturing event listener to the input, which theoretically helps ensure that the masking code executes before normal vue event handlers (which do not specify useCapture: true). Chrome and Edge correctly execute the directive's input handler first, but Firefox does not.

Simple demonstration of the issue:

Edge (chromium) image

Firefox image

I haven't looked too much further into this yet, but I did make a quick change to the directive to add the input event handler on the input element's parent, and the issue in Firefox went away.

Basic explanation of capture/bubble event phases: https://www.quirksmode.org/js/events_order.html

Here is a codepen that can be used to quickly assess the capture/bubble: https://codepen.io/taylorcognito/pen/JjEeOGe?editors=1010

Checklist

tskimmett commented 3 years ago

I managed to find this issue in Firefox's bug tracking site.

RonaldJerez commented 3 years ago

Hello, thanks for the PR. I'll review soon and merge/leave feedback.

RonaldJerez commented 3 years ago

@tskimmett Change set looks good. Can you amend the commit message to use commitizen format. With the current commit messages no new release will be generated as there is no fix commit.

tskimmett commented 3 years ago

@tskimmett Change set looks good. Can you amend the commit message to use commitizen format. With the current commit messages no new release will be generated as there is no fix commit.

The PR title looks to be formatted correctly. Can't you just do a squash merge, which takes the PR title as the commit message?

RonaldJerez commented 3 years ago

@tskimmett Change set looks good. Can you amend the commit message to use commitizen format. With the current commit messages no new release will be generated as there is no fix commit.

The PR title looks to be formatted correctly. Can't you just do a squash merge, which takes the PR title as the commit message?

I can try that then, I was not aware that is how github did it.

tskimmett commented 3 years ago

Yep, when you click "Squash Merge" it actually shows you the commit message, and you can also modify it right there if you need.

RonaldJerez commented 3 years ago

:tada: This PR is included in version 1.3.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

tskimmett commented 3 years ago

@RonaldJerez Thanks for getting this in quickly!

RonaldJerez commented 3 years ago

No problem. Thanks for looking into the issue.