Skip to content

work around chrome 85 scrollLeft bug#6

Merged
alitaheri merged 1 commit into
alitaheri:masterfrom
Cansn0w:master
Sep 7, 2020
Merged

work around chrome 85 scrollLeft bug#6
alitaheri merged 1 commit into
alitaheri:masterfrom
Cansn0w:master

Conversation

@Cansn0w

@Cansn0w Cansn0w commented Sep 3, 2020

Copy link
Copy Markdown
Contributor

This change intends to work around a bug introduced by Chrome version 85. In this version Chrome now implemented the standard/negative scrollLeft calculation, but it also brought in a bug where scrollLeft of an RTL container could be at maximum 1 whereas the maximum should be 0 in this case. It may be related to this reported bug https://bugs.chromium.org/p/chromium/issues/detail?id=1123301

This works around the bug by setting scrollLeft to 2 and then compare it against 2.

The following screenshots are taken when debugging this library on the same overflowing RTL container, demonstrating the problem and how the fix works around it. (Chrome Version 85.0.4183.83 on MacOS Version 10.15.6)
Screen Shot 2020-09-02 at 8 17 27 pm

Screen Shot 2020-09-03 at 2 41 18 pm

@Cansn0w

Cansn0w commented Sep 3, 2020

Copy link
Copy Markdown
Contributor Author

@alitaheri feel free to make necessary changes before merging and let me know if anything else I can help with 😄

@alitaheri

Copy link
Copy Markdown
Owner

@Cansn0w Thank you very much for the contribution. I only have one concern. Won't this break older versions of chrome? until everyone upgrades this will break stuff, also, if this bug gets fixed after a future update, that will also break it. However, if it works in a way that doesn't break when this bug is fixed or for older versions then it's good. It's just that I've wrote this code a long time ago and don't remember much. I will have to take a deeper look. Either confirm that this change won't break other cases or wait for me to look into it. Thank you 🙏

@Cansn0w

Cansn0w commented Sep 7, 2020

Copy link
Copy Markdown
Contributor Author

Thank you for your response and continuous maintenance of the library!

To my knowledge this should not break in browsers without this bug, e.g. older or future version of Chrome. The change proposed in this PR is similar to how we used to solve the problem, that is, assigning a positive value to scrollLeft and see if the browser changes it, since only negative scrollLeft strategy would change it. In this PR we work around the off-by-1 bug by assigning dummy.scrollLeft = 2; so we're now testing if scrollLeft is in the range of [-scrollWidth, 1] instead of [-scrollWidth, 0].

but please do take a look in case there could be other problems 😄

@alitaheri

Copy link
Copy Markdown
Owner

@Cansn0w uh, right. you're on point. Thanks for looking into this issue. I will publish a new version asap after I test it out myself 👍

@alitaheri
alitaheri merged commit cf92b71 into alitaheri:master Sep 7, 2020
@alitaheri

Copy link
Copy Markdown
Owner

@Cansn0w published as v0.2.1

@Cansn0w

Cansn0w commented Sep 9, 2020

Copy link
Copy Markdown
Contributor Author

Thank you!

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.

2 participants