| SBisson | |
| Dec 17 2018, 9:03 PM |
| F29260407: fullscreen.gif | |
| May 27 2019, 10:05 PM |
| F29260409: small.gif | |
| May 27 2019, 10:05 PM |
| F27930692: 99EAAA4C-0F91-4217-849A-8EEFEAA14F43.png | |
| Jan 17 2019, 9:51 PM |
| F27930691: 2F562851-56FB-4227-AD31-E4EACB2EAF1E.png | |
| Jan 17 2019, 9:51 PM |
| F27904587: B6989F60-3FCA-4D1C-AEE1-57BC84AA3814.png | |
| Jan 16 2019, 1:29 AM |
| F27904588: 8C25346E-617E-421E-A3BE-6AF79234F982.png | |
| Jan 16 2019, 1:29 AM |
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.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | matmarex | T221328 [Epic] Mobile VE usability improvements (FY18-19 Q4) | |||
| Resolved | matmarex | T227271 Dialog footers should be moved up on iOS when navigation menu isn't visible. | |||
| Resolved | matmarex | T212159 Scrolling a ProcessDialog scrolls the page behind on iOS Safari |
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.
OOUI also disables scrolling on the body using overflow: hidden;, but clearly iOS Safari is ignoring that.
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
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:
Change 480580 merged by jenkins-bot:
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog
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"
Change 480692 merged by jenkins-bot:
[oojs/ui@master] Revert "WindowManager: Prevent iOS Safari from scrolling the page behind the dialog"
Change 481239 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Update OOUI to v0.30.0
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)
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.
There are two approaches to preventing page scrolling on iOS:
(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.
 |  |
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)
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.
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:
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.)
| Before | After |
|---|---|
 |  |
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)
Change 485111 abandoned by Bartosz Dziewoński:
WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)
Reason:
See task
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.
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.
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:
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: |
|---|---|
| |
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)
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.
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.
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.)
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.
Change 485111 merged by jenkins-bot:
[oojs/ui@master] WindowManager: Prevent iOS Safari from scrolling the page behind the dialog (try#2)
Change 523823 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/core@master] Update OOUI to v0.33.3
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
Change 524063 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: Compat for OOUI scrolling fix on iOS Safari
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: