Skip to content

Conversation

@smfr
Copy link
Contributor

@smfr smfr commented Aug 18, 2022

98069e5

scrollIntoView() does not work on option elements
https://bugs.webkit.org/show_bug.cgi?id=243517
<rdar://98475444>

Reviewed by Ryosuke Niwa.

<option> and <optgroup> are special because they don't have their own renderers;
they get painted as items by their enclosing <select> element.

Because of this `scrollIntoView()` on these elements did nothing, since `Element::scrollIntoView()`
returned early if there is no renderer.

Fix in a similar way to how `Element::getClientRects()` is implemented: add special
handling when the element is an HTMLOptionElement or HTMLOptGroupElement. `listBoxElementScrollIntoView()`
first handles the scroll in the <select>, then allows the existing call to `FrameView::scrollRectToVisible()`
to scroll the enclosing scrollers.

* LayoutTests/fast/scrolling/scrollIntoView-on-option-expected.txt: Added.
* LayoutTests/fast/scrolling/scrollIntoView-on-option.html: Added.
* LayoutTests/platform/ios-wk2/fast/scrolling/scrollIntoView-on-option-expected.txt: on iOS, the
<select> is rendered as a menu list, so the <option> has no visible representation to scroll to,
and no scrolling occurs. This matches the behavior for a menulist on macOS in all browsers.
* Source/WebCore/dom/Element.cpp:
(WebCore::listBoxElementScrollIntoView):
(WebCore::Element::scrollIntoView):

Canonical link: https://commits.webkit.org/253585@main

f256ac0

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ❌ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ❌ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios 🧪 api-mac ❌ 🧪 api-gtk
✅ 🛠 tv 🧪 mac-wk1
✅ 🛠 tv-sim ❌ 🧪 mac-wk2
✅ 🛠 🧪 merge ✅ 🛠 watch 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@smfr smfr requested review from cdumez and rniwa as code owners August 18, 2022 05:22
@smfr smfr self-assigned this Aug 18, 2022
@smfr smfr added CSS Cascading Style Sheets implementation Safari 15 labels Aug 18, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 18, 2022
@smfr smfr removed the merging-blocked Applied to prevent a change from being merged label Aug 18, 2022
@smfr smfr force-pushed the eng/scrollIntoView-does-not-work-on-option-elements branch from 4bebe69 to f256ac0 Compare August 18, 2022 17:13
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 18, 2022
@smfr smfr added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Aug 19, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 98069e5 into WebKit:main Aug 19, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 253585@main (98069e5): https://commits.webkit.org/253585@main

Reviewed commits have been landed. Closing PR #3438 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/scrollIntoView-does-not-work-on-option-elements branch from f256ac0 to 98069e5 Compare August 19, 2022 07:06
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants