jondavidjohn / payform

:credit_card: A library for building credit card forms, validating inputs, and formatting numbers.
https://jondavidjohn.github.io/payform/
Other
427 stars 81 forks source link

Cannot assign to read only property 'target' of object KeyboardEvent and InputEvent #37

Closed ddayguerrero closed 6 years ago

ddayguerrero commented 6 years ago

What is the issue?

Validation and formatting are not triggered because of uncaught errors thrown in the _eventNormalize function on keyboard and input events. There is an attempt to assign values to readonly properties.

This occurs on latest Chrome, Firefox, and Safari.

_eventNormalize = function(listener) {
      return function(e) {
        if (e == null) {
          e = window.event;
        }
        e.target = e.target || e.srcElement; // Uncaught TypeError thrown here
        e.which = e.which || e.keyCode; // Uncaught TypeError thrown here too
        if (e.preventDefault == null) {
          e.preventDefault = function() {
            return this.returnValue = false;
          };
        }
        return listener(e);
      };
    };

Error Log:

Uncaught TypeError: Cannot assign to read only property 'target' of object '#<KeyboardEvent>'
    at HTMLInputElement.<anonymous> (payform.js:43)
(anonymous) @ payform.js:43
payform.js:43 Uncaught TypeError: Cannot assign to read only property 'target' of object '#<InputEvent>'
    at HTMLInputElement.<anonymous> (payform.js:43)

How to reproduce the issue?

Discussion

This behaviour only occurs when vendoring the distribution file. No errors are thrown if we install the package via npm and use require.

From what I understand, the _eventNormalize function was introduced to modify the event for legacy browser support (e.g. IE8).

According to MDN, the keyCode attribute is deprecated and should not be used when handling the keyboard event types (e.g. keydown) handled in payform.

Here is what I suggest to allow users to vendor payform: 1- Recreate a new event and assign target and which attributes 2- Drop support for legacy browsers?

What do you think? 🙂

jondavidjohn commented 6 years ago

I actually don't mind the idea of creating a synthetic event to normalize the property access, maybe we could just return a re-created event object from _eventNormalize?

jondavidjohn commented 6 years ago

In addition to target and which, it looks like we also call prevent default in some of the listeners so we'd need to also add that to the mocked event.

jondavidjohn commented 6 years ago

Fixed @ 1.2.5