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

fix(cdk/platform): rtl scroll axis incorrectly determined in Safari macOS #19830

Merged

Conversation

devversion
Copy link
Member

@devversion devversion commented Jul 1, 2020

In Safari on macOS, the RTL scroll axis is determined incorrectly when
scrollbars are set to Always Display as per macOS general system
preferences. Our test element for the scroll axis sets a fixed height.

This is resulting in a vertical scroll bar as the horizontal scroll bar
for testing scrollLeft exceeds the scroll containers 1px height. The
vertical scroll bar then unveils a bug in Webkit where space is acquired
on the right side for the scroll bar (while it's displayed on the left).

This space causes our scroll axis detection to break as the horizontal
scrollLeft unexpectedly expands to: [-scrollWidth, 15px] while
usually 0px is the right boundary. We fix this by simply ensuring
that no vertical scroll bar is ever displayed. The vertical scrolling
is not needed for determining the RTL horizontal scroll axis type.

I've reported a bug upstream in Webkit:
https://bugs.webkit.org/show_bug.cgi?id=213851.

I tested manually on Chrome 83.0.4103.116 Windows 10; macOS High Sierra Firefox 77; macOS High Sierra Safari 11.1, macOS Mojave Safari 12.1, macOS Catalina Safari 13.1, Windows 10 IE11.900.18362.0. Unfortunately there are no tests for this feature detection, nor would it be easy to write one for the particular scenario where a macOS setting is required.

Fixes #14609.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…acOS

In Safari on macOS, the RTL scroll axis is determined incorrectly when
scrollbars are set to `Always Display` as per macOS general system
preferences. Our test element for the scroll axis sets a fixed height.

This is resulting in a vertical scroll bar as the horizontal scroll bar
for testing `scrollLeft` exceeds the scroll containers `1px` height. The
vertical scroll bar then unveils a bug in Webkit where space is acquired
on the right side for the scroll bar (while it's displayed on the left).

This space causes our scroll axis detection to break as the horizontal
`scrollLeft` unexpectedly expands to: `[-scrollWidth, 15px]` while
usually `0px` is the right boundary. We fix this by simply ensuring
that no vertical scroll bar is ever displayed. The vertical scrolling
is not needed for determining the RTL horizontal scroll axis type.

I've reported a bug upstream in Webkit:
https://bugs.webkit.org/show_bug.cgi?id=213851.

Fixes angular#14609.
@devversion devversion requested a review from jelbourn as a code owner July 1, 2020 17:47
@devversion devversion added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Jul 1, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmalerba mmalerba added lgtm action: merge The PR is ready for merge by the caretaker labels Jul 1, 2020
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@devversion
Copy link
Member Author

@googlebot Ping.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 2, 2020
@mmalerba mmalerba merged commit 0ddad07 into angular:master Jul 11, 2020
mmalerba pushed a commit that referenced this pull request Jul 11, 2020
…acOS (#19830)

In Safari on macOS, the RTL scroll axis is determined incorrectly when
scrollbars are set to `Always Display` as per macOS general system
preferences. Our test element for the scroll axis sets a fixed height.

This is resulting in a vertical scroll bar as the horizontal scroll bar
for testing `scrollLeft` exceeds the scroll containers `1px` height. The
vertical scroll bar then unveils a bug in Webkit where space is acquired
on the right side for the scroll bar (while it's displayed on the left).

This space causes our scroll axis detection to break as the horizontal
`scrollLeft` unexpectedly expands to: `[-scrollWidth, 15px]` while
usually `0px` is the right boundary. We fix this by simply ensuring
that no vertical scroll bar is ever displayed. The vertical scrolling
is not needed for determining the RTL horizontal scroll axis type.

I've reported a bug upstream in Webkit:
https://bugs.webkit.org/show_bug.cgi?id=213851.

Fixes #14609.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getRtlScrollAxisType detects incorrect RtlScrollAxisType for safari
4 participants