Page MenuHomePhabricator

[Regression 1.28.0-wmf.15] Programmatic document change (e.g. make bold) causes scroll to top in IE<=11
Closed, ResolvedPublic1 Story Points

Description

  1. Open a largish page
  2. Scroll down a bit and select some text
  3. Press ctrl+b

You are scrolled back to the top of the page

Details

Related Gerrit Patches:
mediawiki/extensions/VisualEditor : masterUpdate VE core submodule to master (33012af)
VisualEditor/VisualEditor : masterce.Surface showSelectionState: Avoid unnecessary refocusing

Event Timeline

Esanders created this task.Nov 10 2016, 5:14 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 10 2016, 5:14 PM
Esanders reopened this task as Open.Nov 10 2016, 6:09 PM

That commit was for a different bug

Esanders added a subscriber: DLynch.EditedNov 10 2016, 6:10 PM

blame says this is the offending commit: https://gerrit.wikimedia.org/r/#/c/304575/ @DLynch

Esanders reassigned this task from Esanders to DLynch.Nov 11 2016, 10:06 AM
Jdforrester-WMF renamed this task from [Regression] Programmatic document change (e.g. make bold) causes scroll to top in IE<=11 to [Regression 1.28.0-wmf.15] Programmatic document change (e.g. make bold) causes scroll to top in IE<=11.Nov 14 2016, 7:12 PM

Can you see what is going on here @DLynch - I couldn't quite work out what case your original code was fixing.

DLynch added a comment.Dec 5 2016, 5:03 PM

My code was fixing an issue most easily seen with long references containing large blocks of text. Pressing backspace in them would scroll the reference dialog all the way to the top, because of the on-backspace selection fixing code reapplying the selection.

Change 325960 had a related patch set uploaded (by DLynch):
ce.Surface showSelectionState: Avoid unnecessary refocusing

https://gerrit.wikimedia.org/r/325960

DLynch added a comment.Dec 8 2016, 5:46 PM

Issue was: in IE<=11, focusing an already-focused element will scroll the viewport to bring the top edge of the element into view. Other browsers skip this, if the element is already visible in the viewport. Edge brings its behavior in-line with the other browsers.

The attached patch just checks whether the currently active element is the element that would be focused, and skips doing anything if so. (Scrolling to the new selection from off-screen if the element is focused and you've scrolled away happens in all current browsers without the focus being required.)

Change 325960 merged by jenkins-bot:
ce.Surface showSelectionState: Avoid unnecessary refocusing

https://gerrit.wikimedia.org/r/325960

Change 326141 had a related patch set uploaded (by Jforrester):
Update VE core submodule to master (9be803f)

https://gerrit.wikimedia.org/r/326141

Jdforrester-WMF closed this task as Resolved.Dec 10 2016, 12:09 AM
Jdforrester-WMF removed a project: Patch-For-Review.

Change 326141 merged by jenkins-bot:
Update VE core submodule to master (33012af)

https://gerrit.wikimedia.org/r/326141

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMar 21 2017, 7:26 PM