It does nothing, e.g. OO.ui.Element.static.scrollIntoView($('#footer')[0])
I tested against the demos which blames https://gerrit.wikimedia.org/r/#/c/oojs/ui/+/525634/, which is a commit only for the demos.
Edit: Bisecting Vector found a similar commit: https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/551672/
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Esanders | T252903 Page scrolls to the wrong place after posting a comment | |||
| Resolved | Esanders | T255162 OO.ui.Element.static.scrollIntoView is broken |
Change 604786 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/skins/Vector@master] Revert "Set overflow-y to scroll to prevent reflow"
Change 604787 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/skins/MinervaNeue@master] Revert "Set overflow-y to scroll to avoid reflow"
The correct fix will be to patch scrollIntoView, but for now we should revert the offending patches. That method being broken seems more critical than T204084.
Change 604793 had a related patch set uploaded (by Esanders; owner: Esanders):
[oojs/ui@master] Revert "demos: Make forced scrolling rule compatible with disabling scroll for dialogs"
Can you expand the description with some reproduction steps. I have no idea what this is about right now (and why it should be considered UBN).
Ahem, we would need to expect as lib to be in an environment where overflow-y: scroll is set…?!
Or did I misread your comment above @Esanders and you mean patching scrollIntoView to take overflow-y into account?
Or did I misread your comment above @Esanders and you mean patching scrollIntoView to take overflow-y into account?
That's correct. Edit: To be clear I would happily abandon these reverts if there was a fix for scrollIntoView, but given that is of unknown complexity right now we should revert the regression while we figure it out.
Can you expand the description with some reproduction steps.
Sure, a quick command line test is OO.ui.Element.static.scrollIntoView($('#footer')[0])
Anywhere you see scrollIntoView in our code would be broken. We use to it keep templates and inspectors in the viewport, amongst other things.
Change 604936 had a related patch set uploaded (by Catrope; owner: Catrope):
[oojs/ui@master] Element: Deal with scrollable <body> in scrollIntoView()
Change 605190 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/core@master] Add temporary workaround for scrollIntoView bug when OOUI is loaded
Change 604786 abandoned by Esanders:
Revert "Set overflow-y to scroll to prevent reflow"
Reason:
See I34c05c512d731eb5977c51f503f7f29ebcdfbdbf
Change 604787 abandoned by Esanders:
Revert "Set overflow-y to scroll to avoid reflow"
Reason:
See I34c05c512d731eb5977c51f503f7f29ebcdfbdbf
Per @Jdlrobson 's suggestion, https://gerrit.wikimedia.org/r/605190 only applies the revert when OOUI is loaded, so it doesn't affect most readers on Minerva, which I think was the initial case for the fix.
Change 605207 had a related patch set uploaded (by Esanders; owner: Esanders):
[oojs/ui@master] Element: Fix getClosestScrollableContainer when body has overflow
Change 604936 abandoned by Bartosz Dziewoński:
Element: Deal with scrollable <body> in scrollIntoView()
Reason:
Merging Ed's patch instead.
Change 605207 merged by jenkins-bot:
[oojs/ui@master] Element: Fix getClosestScrollableContainer when body has overflow
Change 604793 abandoned by Bartosz Dziewoński:
Revert "demos: Make forced scrolling rule compatible with disabling scroll for dialogs"
Reason:
No longer needed.
That should fix the underlying issue.
Do we still want to merge the various workarounds, or just wait for the next OOUI release?
Change 605190 abandoned by Esanders:
Add temporary workaround for scrollIntoView bug when OOUI is loaded
Reason:
Ib6bda230f3ab4f0a12454cc7eafc220ce5f7f700 has been merged into OOUI
Change 607443 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Update OOUI to v0.39.2