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

Catch exceptions while assigning event.timeStamp #298

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

briganti
Copy link

@briganti briganti commented Jan 8, 2020

Summary

On some old browsers such as Safari mobile 9, Firefox 52 or Chrome mobile 58, trying to set a read-only property raises an exception TypeError: setting a property that has only a getter.

This has been fixed on the Zepto repository, in this PR. But since no release has been done, it's not available via Semver.

This PR applies the above patch to the versioned Zepto file.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.959% when pulling f7cf20e on briganti:zepto-event-timestamp into 906d8b7 on algolia:master.

@Haroenv
Copy link
Contributor

Haroenv commented Jan 9, 2020

This looks like a valid fix, thanks! Do you have a broken app, so we can confirm what behaviour exactly changed? Thanks!

@briganti
Copy link
Author

briganti commented Jan 9, 2020

I've more info about this error. On these old browsers, event.timeStamp is set incorrectly to 0 (instead of the Epoch time it was created). This is what I've got on FF 52:

image

This causes the following code:

https://github.com/algolia/autocomplete.js/blob/906d8b7fedfa9a203b7d05a2daeffdc190b5304c/zepto.js#L1080

to assign event.timeStamp to the current time. But there is one problem, event.timeStamp cannot be changed - it's a read-only property. Trying to do so will raise a TypeError exception.

I was struggling to reproduce this error on the playground page. Until I notice that the Zepto library is not using strict mode. That's why the TypeError exception does not appear in the console logs, it got ignored by the browser.

But in my app, autocomplete.js lib is part of a strict mode bundle.

Now, I can set the Zepto file in strict mode (that'll make the error shows up on the playground page), but that might cause some other exceptions to pop up. Wdyt @Haroenv

@Haroenv Haroenv merged commit 652bde4 into algolia:master Jan 10, 2020
@briganti briganti deleted the zepto-event-timestamp branch February 12, 2020 22:28
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

Successfully merging this pull request may close these issues.

None yet

3 participants