Page MenuHomePhabricator

In mobile VE, using the "Format" menu causes the page to scroll up by ~half the screen every time (iOS only?)
Closed, ResolvedPublic

Description

In mobile VE, using the "Format" menu causes the page to scroll up by ~half the screen every time. Only tested on iOS so far, no idea what causes this and if it may affect other devices.

Video: https://drive.google.com/file/d/1NXZkJwQBW_IvD6ZZAuwBj-DbcBoXZMBk/view (too large for Phabricator)

I thought this may be related to T218635, but the patch for that bug does not fix it.

Event Timeline

matmarex renamed this task from In mobile VE, using the "Format" popup causes the page to scroll up by ~half the screen every time (iOS only?) to In mobile VE, using the "Format" menu causes the page to scroll up by ~half the screen every time (iOS only?).Mar 19 2019, 11:20 AM
matmarex updated the task description. (Show Details)

Change 498650 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.ui.Surface: Maybe fix #scrollCursorIntoView for iOS

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

Change 498910 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.ui.Surface: Call #scrollCursorIntoView in some more cases

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

Root cause of this is an iOS Safari bug, not a problem in our code (Safari just scrolls the page to completely the wrong place when we restore the selection after closing the toolbar menu). My patches attempts to override this, and at a glance, they seem to work.

However, it would take us quite some effort to verify that this doesn't mess with the scrolling in other situations. I don't really want to spend too much time on this without talking about it. I'll abandon this for now.

Change 498650 abandoned by Bartosz Dziewoński:
ve.ui.Surface: Maybe fix #scrollCursorIntoView for iOS

Reason:
See task

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

Change 498910 abandoned by Bartosz Dziewoński:
ve.ui.Surface: Call #scrollCursorIntoView in some more cases

Reason:
See task

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

To verify that it's a Safari bug: place a breakpoint in ve.ce.Surface.prototype.showSelectionState on the line sel.addRange( newSel.getNativeRange( this.getElementDocument() ) ); and step over it, it just scrolls the selection straight out of view.

Can we just store the document scroll offset and restore it when the menu is closed?

I didn't think of that, I guess we can… it causes a slightly ugly scrolling effect, but we can live with that I guess.

Change 499960 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.init.Target: Allow overriding surface (de)activation on mobile

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

Change 499961 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Save/restore scroll position on surface (de)activation

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

Change 499960 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.init.Target: Allow overriding surface (de)activation on mobile

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

Change 500087 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (549f49bb5)

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

Change 500087 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (549f49bb5)

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

Change 499961 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Save/restore scroll position on surface (de)activation

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

@matmarex: so this scrolling issue seems fixed. However, T218635 still is not.

ppelberg claimed this task.