Page MenuHomePhabricator

Scrolling a ProcessDialog scrolls the page behind on iOS Safari
Closed, ResolvedPublic

Description

Reported by @MMiller_WMF at T207717#4815994

When viewing a ProcessDialog on mobile (full screen), trying to scroll it actually scrolls the page in the background, as can be seen by the scrollbar movement.

It was experienced on a specific dialog we are working on but later confirmed using the demo page.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 17 2018, 9:03 PM

I can reproduce this in the demos, but it works correctly when using mobile VisualEditor. We should figure out what exactly is done in MobileFrontend or in VisualEditor to fix this problem and upstream that fix to OOUI. @Jdlrobson Do you happen to know off the top of you head?

We disable scrolling on the body using an overlay-enabled class and store the scroll position

.overlay-enabled #mw-mf-page-center {
    overflow: hidden;
    display: block;
}

When the overlay is closed we restore the scroll position.

Volker_E moved this task from Backlog to Next-up on the OOUI board.Dec 18 2018, 3:41 AM
Volker_E triaged this task as High priority.Dec 18 2018, 7:42 AM

OOUI also disables scrolling on the body using overflow: hidden;, but clearly iOS Safari is ignoring that.

matmarex claimed this task.Dec 18 2018, 8:05 PM

Change 480580 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog

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

I found what VE does to fix this and implemented the same thing in OOUI.

Also, slightly related patches I wrote while working on this:

Volker_E moved this task from Next-up to OOUI-0.30.0 on the OOUI board.Dec 18 2018, 9:59 PM
Volker_E edited projects, added OOUI (OOUI-0.30.0); removed OOUI.

Change 480580 merged by jenkins-bot:
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog

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

I think I was too hasty here… The iOS hack causes the scroll position on the page to be reset to the top when opening the dialog (and never restored).

Change 480692 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] Revert "WindowManager: Prevent iOS Safari from scrolling the page behind the dialog"

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

Change 480692 merged by jenkins-bot:
[oojs/ui@master] Revert "WindowManager: Prevent iOS Safari from scrolling the page behind the dialog"

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

Change 481239 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Update OOUI to v0.30.0

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

Change 481239 merged by jenkins-bot:
[mediawiki/core@master] Update OOUI to v0.30.0

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

matmarex edited projects, added OOUI; removed OOUI (OOUI-0.30.0).Dec 24 2018, 8:48 AM

(This was reverted and not part of OOUI 0.30.0.)

Also no patch for review?

Volker_E moved this task from Backlog to Next-up on the OOUI board.Jan 7 2019, 6:52 AM

Change 484608 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog ('touchmove' preventDefault)

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

Change 484608 abandoned by Bartosz Dziewoński:
WindowManager: Prevent iOS Safari from scrolling the page behind the dialog ('touchmove' preventDefault)

Reason:
Looks like this won't work, see the task.

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

There are two approaches to preventing page scrolling on iOS:

  1. Setting the height of the document so that there is nothing to scroll (the one I tried before)
  2. Preventing the default handling of 'touchmove' event so that scrolling never happens

(Of course none of that would be necessary if iOS Safari correctly supported overflow: hidden on <body>.)


Since the first approach turned out to not be trivial, I spent some time trying the second one. It was actually looking great, until I found out that another crazy iOS Safari behavior makes it impossible.

When the page is in "fullscreen mode" (the menu bar is minimized), tapping near the bottom of the screen will display the menu bar at the top and some tools at the bottom. But the way it is implemented means that there are no events fired for touches that begin in that area. So we can't prevent the scroll if the user starts by touching that part of the screen. I think that's a deal-breaker.

When the page is in "fullscreen mode" (the menu bar is minimized), tapping near the bottom of the screen will display the menu bar at the top and some tools at the bottom. But the way it is implemented means that there are no events fired for touches that begin in that area. So we can't prevent the scroll if the user starts by touching that part of the screen. I think that's a deal-breaker.

For clarity and reference, here are some articles that rant about this and explain the problem better than I did:

Change 484840 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (fake scroll)

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

Change 484840 abandoned by Bartosz Dziewoński:
WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (fake scroll)

Reason:
I'm not happy with this either, see the task.

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

Going back to the first approach. The obvious solution would be to just restore the scroll position after the dialog is closed. But with small dialogs (not fullscreen), it would look weird, since the original page is visible through the translucent dialog background – so the user could see we're messing with the scroll position.

To avoid this, I experimented with faking the scroll by using margins/positioning/transforms on the <body> element. It kind of works (the patch uses margins), but:

  • When applying the new styles, the whole page "flashes" for a bit on iOS Safari. (I didn't see the effect when testing the code using the device toolbar in desktop Chrome.)
  • The page is still scrolled to the top in reality, so the fake effect is not perfect. For example desktop VE's sticky toolbar is mispositioned while a dialog is open. This would mean that we would have to only use this mode on iOS, and this in turn would no doubt result in issues when code doing stuff with dialogs that was only tested on desktop would break on mobile (or vice versa). I would prefer a solution that could also harmlessly be enabled on desktop.
  • From a long-term maintenance perspective, applying weird styles like this on <body> is a bit scary. It seems like just begging for crazy rendering issues or weird incompatibilities with our other code.

Going back to the first approach. The obvious solution would be to just restore the scroll position after the dialog is closed. But with small dialogs (not fullscreen), it would look weird, since the original page is visible through the translucent dialog background – so the user could see we're messing with the scroll position.

And from this, another obvious (if inelegant) solution follows: just change the translucent background to opaque, on iOS only. I think this is the only thing we can do. (Other than just accepting that iOS users will accidentally scroll the page behind the dialog, and documenting this as a browser bug with incorrect handling for body { overflow: hidden; } – which also might be a reasonable solution.)

BeforeAfter

Note that this is less awful than it looks because in practice, most dialogs on mobile devices will be fullscreen. It will mostly only look ugly on iPads and such, and in silly examples in the demo like the one above.

Change 485111 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)

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

kostajh added a subscriber: kostajh.Feb 1 2019, 3:07 AM

Change 485111 abandoned by Bartosz Dziewoński:
WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)

Reason:
See task

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

matmarex removed matmarex as the assignee of this task.Feb 19 2019, 11:17 PM
matmarex removed a project: Patch-For-Review.
matmarex moved this task from Next-up to Backlog on the OOUI board.

Lukewarm responses to my patch convince me that we all feel like the solutions are worse than the problem. I am not planning to work on this further. Please read my earlier comments for the drawbacks of each possible solution.

matmarex renamed this task from Scrolling a ProcessDialog scrolls the page behind on iphone 6S/iOS12.1.1/Safari to Scrolling a ProcessDialog scrolls the page behind on iOS Safari.Mar 15 2019, 6:08 PM

Since we ditched all iOS hacks last week [https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/496827], this issue now affects VisualEditor. Maybe I'll be able to get someone to give this some thought.

matmarex claimed this task.May 23 2019, 9:15 PM

I'm still working on my promised summary of the potential solutions and their drawbacks.

In the meantime, I've accidentally learned that the Safari bug causing us all this woe (Bug 153852 - <body> with overflow:hidden CSS is scrollable on iOS) has been resolved a few days ago! This adds another viable potential solution: just wait until a Safari version including it is released, and blacklist older versions. Unfortunately I have no idea when such release might happen.

Also, slightly related commit: https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/512248 (removes non-working code attempting to fix this in VE).

Here's a summary, rephrasing my earlier comments in a more logical order and with some corrections.

Problem:

When viewing a dialog on iOS, trying to scroll it can scroll the page in the background, as can be seen by the scrollbar movement. This is more than a small visual annoyance, because the browser seems to "remember" which element is being scrolled, and trying to scroll actually scrollable content inside the dialog afterwards may instead continue scrolling the page in the background.

This is caused by a bug in iOS Safari: it incorrectly handles body { overflow: hidden; } in CSS, which is supposed to prevent scrolling of the page (https://www.w3.org/TR/css-overflow-3/#valdef-overflow-hidden UA must not provide any scrolling user interface to view the content outside the clipping region, nor allow scrolling by direct intervention of the user, such as dragging on a touch screen or using the scrolling wheel on a mouse).

Possible solutions:

Approach 0: Do nothing, wait for Safari bug to be fixed

The Safari bug causing us all this woe (Bug 153852 - <body> with overflow:hidden CSS is scrollable on iOS) has been resolved recently. This means we could just wait until a Safari version including the fix is released; unfortunately I have no idea when such release might happen.

We might then blacklist older versions, or just consider it a known problem and tell anyone who asks that it's a browser bug.)

Approach 1: Prevent scrolling by setting the height of the document so that there is nothing to scroll (https://gerrit.wikimedia.org/r/c/oojs/ui/+/480580)

The drawback of this approach is that when there is nothing to scroll, Safari's menu bar is never minimized, and so we reduce the amount of screen space available for the dialog's contents (see screenshots below).

It also causes the scroll position on the page to be reset to the top when opening the dialog, and we need to restore it after the dialog is closed. This looks weird with small dialogs (not fullscreen), since the original page is visible through the translucent dialog background – so the user could see the page scroll to the top and then back down. There are two ways to solve this sub-problem:

Approach 1.1: Fake the scroll by using margins/positioning/transforms on the <body> element (https://gerrit.wikimedia.org/r/c/oojs/ui/+/484840)

It kind of works (the patch uses margins), but:

  • When applying the new styles, the whole page "flashes" for a bit on iOS Safari. (I didn't see the effect when testing the code using the device toolbar in desktop Chrome.)
  • The page is still scrolled to the top in reality, so the fake effect is not perfect. For example desktop VE's sticky toolbar is mispositioned while a dialog is open. This means that we would have to only use this mode on iOS, and this in turn could result in issues when code doing stuff with dialogs that was only tested on desktop would break on mobile (or vice versa). I would prefer a solution that could also harmlessly be enabled on desktop.
  • From a long-term maintenance perspective, applying weird styles like this on <body> is a bit scary. It seems like just begging for crazy rendering issues or weird incompatibilities with our other code.

Approach 1.2: Change the translucent background to opaque, on iOS only (https://gerrit.wikimedia.org/r/c/oojs/ui/+/485111)

This is the safest, but rather inelegant.

Note that this is less awful than it looks because in practice, most dialogs on mobile devices will be fullscreen. It will mostly only look ugly on iPads and such, and in silly examples in the demo like the one above.

Approach 2: Prevent the default handling of touch events so that scrolling never happens (https://gerrit.wikimedia.org/r/c/oojs/ui/+/484608)

When the page is in "fullscreen mode" (the menu bar is minimized; see screenshots below), tapping near the bottom of the screen will display the menu bar at the top and some tools at the bottom. But the way it works is that there are no events fired for touches that begin in that area. So we can't prevent the scroll if the user starts by touching that part of the screen.

Here are some articles that rant about this and explain the problem better:

Summary: Of the two ways to work around the Safari bug, one forces us to reduce the screen space for the dialog, and the other doesn't work when you tap on a specific part of the screen. They both come with additional complexity, and overall may be worse than the problem.


Screenshots for reference:

Browser menu bar minimized:Browser menu bar expanded, with tools:
JTannerWMF added subscribers: Esanders, JTannerWMF.

Hey @matmarex , to clarify, you're waiting on a decision on an approach from @ppelberg, @Esanders or someone else?

I've been hoping for some feedback, but if no one else has the time to evaluate the possible solutions, then my proposal is that we go with 1.2.

Change 485111 restored by Bartosz Dziewoński:
WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)

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

What are we doing differently in the VE save dialog, because if I view a long diff it scrolls fine?

We don’t do anything differently, and it has the same bug. Try touching the dialog header or one of the buttons, and swiping as if you wanted to scroll. Nothing should happen, but instead, the page behind the dialog will scroll (you will see the scrollbar appear). Now quickly afterwards, try swiping in the dialog to scroll it. Instead of the dialog scrolling, the page behind it will scroll again.

The drawback of this approach is that when there is nothing to scroll, Safari's menu bar is never minimized, and so we reduce the amount of screen space available for the dialog's contents (see screenshots below).

I'm not sure this is much of a problem because that space needs to be kept clear of anything clickable. Having the menu bar always visible solves T227271.

Well, even better then…

Could we apply this to just fullscreen dialogs? If the dialog isn't fullscreen then it will almost certainly not contain scrollable content, so the user will almost never be attempting to scroll in the first place. If they do try scrolling they will still be able to see the page scrolling behind, so although it will seem strange that it is allowed, they will not become disoriented.

Could we apply this to just fullscreen dialogs?

Perhaps, but that at least opens up some new questions, since dialogs can dynamically change from fullscreen to non-fullscreen. (The dialog's code can resize it, and they also can change to the other mode after the user turns their device from portrait to landscape, which will resize the browser viewport.)

  • Should we still restore the scroll position after a non-fullscreen dialog is closed? What if it started as fullscreen?
  • What scroll position should we restore after a non-fullscreen dialog was opened, then the user scrolled, then it changed to fullscreen, then it was closed?
  • Should we apply/remove the scroll prevention code when a dialog changes between non-fullscreen and fullscreen?

I feel like responding to all these questions, and then implementing the answers, is not really a great use of time – because in practice, especially on phones rather than tablets, basically all dialogs will always be fullscreen (certainly this is the case for all dialogs in VE).

I guess that does also mean that we could just pick random answers to those questions and roll with it. It shouldn't really matter either way. And I can implement that if that's what it takes to finally get this merged, but the code is simpler when we use the same behavior for fullscreen and non-fullscreen dialogs.

The abandon changes dialog in VE is pretty common and not fullscreen.

Dialogs rarely change from full screen to not on mobile (afaik) so we should just assume they stay the same size.

We can come back to that if it becomes a common pattern.

Alright, I updated the patch.

Change 485111 merged by jenkins-bot:
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)

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

Change 523823 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/core@master] Update OOUI to v0.33.3

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

Change 523823 merged by jenkins-bot:
[mediawiki/core@master] Update OOUI to v0.33.3

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

matmarex moved this task from Code review to QA on the VisualEditor (Current work) board.

Change 524063 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: Compat for OOUI scrolling fix on iOS Safari

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

Change 524063 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: Compat for OOUI scrolling fix on iOS Safari

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

matmarex moved this task from Inbox to High Priority on the Editing QA board.Aug 9 2019, 4:57 PM
ppelberg closed this task as Resolved.Sep 6 2019, 8:33 PM

This looks great, @matmarex. Documenting some notes from our conversation earlier today confirming how this patch behaves...

We've gone with an approach that does the following:

  • For full-screen dialogs, on iOS:
    • The screen which a dialog appears on top of (call it: underlying screen) will not be visible to a contributor.
    • Not only that, any scrolls the contributor makes within the full screen dialog will only affect that dialog and not the underlying view
  • For non-full-screen dialogs, on iOS:
    • The issue remains. Although, we are ok with this persisting in these circumstances considering we think there is a low-likelihood a contributor will encounter the issue and if they do, not cause them much trouble. See Ed's explanation here: T212159#5337412