Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined is not a function (evaluating 'e.getAttribute("data-iscapture")') #256

Closed
LeoIannacone opened this issue Feb 9, 2017 · 14 comments

Comments

@LeoIannacone
Copy link

This code react-tooltip/dist/decorators/isCapture causes error in Safari (v8 and v9 for sure):

exports.default = function (target) {
  target.prototype.isCapture = function (currentTarget) {
    var dataIsCapture = currentTarget.getAttribute('data-iscapture');
    return dataIsCapture && dataIsCapture === 'true' || this.props.isCapture || false;
  };
};
@wwayne
Copy link
Collaborator

wwayne commented Feb 9, 2017

I just tested on Safari 9.1.3 (11601.7.8), didn't find the error, any clues?

@LeoIannacone
Copy link
Author

LeoIannacone commented Feb 9, 2017

Not really,

I get this error with safari v9 (iOS) and v8 (Mac 10.10)

Everything is fine with tooltip version 3.1.7

@huumanoid
Copy link
Contributor

huumanoid commented Feb 10, 2017

Could you provide more detailed stack trace, please?
Is problem exactly browser specific? Have you tried another one?

Also, is everything OK with the test page in safari (make dev)? If yes, please, provide an example which reproduces this issue.

@LeoIannacone
Copy link
Author

Stack trace:

TypeError: undefined is not a function (evaluating 'e.getAttribute("data-iscapture")')
  at isCapture(~/react-tooltip/dist/decorators/isCapture.js:9:0)
  at isCaptureMode(~/react-tooltip/dist/index.js:214:0)
  at forEach([native code])
  at isCaptureMode(~/react-tooltip/dist/index.js:214:0)
  at isCaptureMode(~/react-tooltip/dist/index.js:214:0)
  at setValueForProperty(~/react-dom/lib/DOMPropertyOperations.js:119:0)
  at exports(~/react-dom/lib/ReactVersion.js:13:0)
  at closeAll(~/react-dom/lib/escapeTextContentForBrowser.js:36:0)
  at perform(~/react-dom/lib/escapeTextContentForBrowser.js:36:0)
  at perform(~/react-dom/lib/escapeTextContentForBrowser.js:36:0)
  at exports(vendor.js:11662:23)
  at exports(vendor.js:11662:23)
  at ? ([native code])
  at closeAll(~/react-dom/lib/escapeTextContentForBrowser.js:36:0)
  at perform(~/react-dom/lib/escapeTextContentForBrowser.js:36:0)
  at batchedUpdates(~/react-dom/lib/ReactMultiChild.js:23:0)
  at exports(vendor.js:11662:23)
  at internalInstance(~/react-dom/lib/ReactUpdateQueue.js:24:0)
  at enqueueSetState(~/react-view-pager/lib/utils.js:23:0)
  at setState(~/animation-bus/lib/animation-bus.js:31:0)
  at onQuoteStoreChange(main.js:42130:364)
  at ? (~/alt/lib/index.js:13:0)
  at forEach([native code])
  at i(~/alt/lib/index.js:7:0)
  at exports(~/babel-runtime/~/core-js/library/modules/_enum-keys.js:5:0)
  at exports(vendor.js:49658:23)
  at _invokeCallback(~/inline-style-prefixer/lib/plugins/flexboxOld.js:12:38)
  at dispatch(~/inline-style-prefixer/lib/plugins/flexboxIE.js:28:0)
  at ? (~/alt/lib/store/StoreMixin.js:143:0)
  at setAppState(~/alt/lib/index.js:292:0)
  at emitChange(~/alt/lib/store/StoreMixin.js:116:0)
  at fn(~/alt/lib/index.js:100:0)
  at c(~/alt/lib/index.js:305:0)
  at actions(~/alt/lib/index.js:322:0)
  at setShouldSendCallback(raven.js:660:4)

Happens only with safari safari v9 (iOS) and v8 (Mac 10.10) (as far as I know).

@tasiek
Copy link

tasiek commented Feb 10, 2017

I got something similar on iPad Safari:
TypeError: e.getAttribute is not a function. (In 'e.getAttribute("data-iscapture")', 'e.getAttribute' is undefined)

The last working version without this bug seems to be 3.2.3, it happens in 3.2.4 and above.

@huumanoid
Copy link
Contributor

Perhaps this is the reason
ef749c3

Please, try to install a hotfix and test it.
npm install github:huumanoid/react-tooltip#issue-256-safari

@tasiek
Copy link

tasiek commented Feb 11, 2017

Works fine with the hotfix!
(Checked on OSX Safari, which also had that error)

@huumanoid
Copy link
Contributor

Thanks for feedback @tasiek. That's great!
Thanks to information provided by @tasiek and @LeoIannacone it was quite easy to localize the bug.

Ok the issue seems clear.
We need to reliably convert NodeList to JS array.

Facebook's approach looks good.

Here is the dist utilizing the approach above.
npm install github:huumanoid/react-tooltip#issue-256-safari-fbjs

What do you think?

I think @gauthierj needs to be informed about this issue (the author of ef749c3)

@diegoarcega
Copy link

sure he needs please!
image

@huumanoid
Copy link
Contributor

@diegoarcega please, try one of the fixes below and let us know the results. It would be great if you try both of them.

This one is just a revert of erroneous commit. It should work. But it's only a temporary fix, we can't completely rely on it.

npm install github:huumanoid/react-tooltip#issue-256-safari

And this is a good candidate to completely fix the problem for all browsers. But it's not tested across the browsers yet.

npm install github:huumanoid/react-tooltip#issue-256-safari-fbjs

@bradyhigginbotham
Copy link

This issue ended up breaking half our site. That's what I get for not setting version ceilings. XD

@diegoarcega
Copy link

diegoarcega commented Feb 15, 2017

@huumanoid I downgraded it to .0 minor version
image

@gauthierj
Copy link

The proposed cross-browsers fix seems OK to us.

Regards,

Gauthier

huumanoid added a commit to huumanoid/react-tooltip that referenced this issue Feb 16, 2017
@wwayne
Copy link
Collaborator

wwayne commented Feb 17, 2017

Really thanks @huumanoid ,should be fixed in 3.2.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants