Page MenuHomePhabricator

Mobile VE toolbar doesn't stay onscreen on iOS 13 when keyboard is open
Closed, ResolvedPublic

Description

Mobile VE toolbar doesn't stay onscreen on iOS 13 when keyboard is open.

Instead, it disappears and then flickers unpredictably as you scroll.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 21 2019, 12:06 AM

Change 526235 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: Limit iOS fixed toolbar workarounds to iOS 12 only

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

ppelberg added a subscriber: ppelberg.EditedSep 24 2019, 3:15 PM

Great spot, @matmarex. I'm encountering the same issue:

When I scroll an article in edit mode, with the keyboard open, on iOS 13, in Safari, the editing toolbar does not reliably come back into view when I stop scrolling as was this case after we merged the patch in T218414.

I think we should investigate a way to properly present the toolbar when a contributor is scrolling the document in edit mode when they keyboard is visible.

Change 526235 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] MobileArticleTarget: Limit iOS fixed toolbar workarounds to iOS 12 only

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

@matmarex, a couple questions triggered by this part of the patch description: With this patch, on devices running those iOS versions, when you open the keyboard, our edit toolbar will not stay fixed at the top of the screen.

Questions

  • 1. After deploying this patch, on iOS 11 and older, how would a user bring the editing toolbar back into view, when the keyboard is open?
  • 2. After this patch, on iOS 12, how would the toolbar behave when the keyboard is open?
  • 3. After this patch, on iOS 13, how would the toolbar behave when the keyboard is open?
  • 1. After deploying this patch, on iOS 11 and older, how would a user bring the editing toolbar back into view, when the keyboard is open?

Scroll up by about half a screen, until the toolbar scrolls into view again (or close the keyboard).

  • 2. After this patch, on iOS 12, how would the toolbar behave when the keyboard is open?

The same as it does now – the user can scroll it out of view, but it will reappear (slide in) after a moment.

  • 3. After this patch, on iOS 13, how would the toolbar behave when the keyboard is open?

The user can scroll it out of view, and it won't reappear, until you scroll up (or close the keyboard).

ppelberg added a comment.EditedOct 18 2019, 9:21 PM
  • 1. After deploying this patch, on iOS 11 and older, how would a user bring the editing toolbar back into view, when the keyboard is open?

Scroll up by about half a screen, until the toolbar scrolls into view again (or close the keyboard).
The same as it does now – the user can scroll it out of view, but it will reappear (slide in) after a moment.

  • 3. After this patch, on iOS 13, how would the toolbar behave when the keyboard is open?

The user can scroll it out of view, and it won't reappear, until you scroll up (or close the keyboard).

What – if anything – can we do to have the toolbar behave in similar ways on iOS13 and iOS 11 as it does on iOS 12? [1]

To me, it's problematic that a contributor could be in a situation where they are not able to see the editing tool [by way of the toolbar] they are trying to use and the text/content they are trying to use that tool to affect at the same time.

Example: https://youtu.be/d5N8JDYhr1k (iPhone XS / iOS 13 / Safari)


  1. iOS 12: ...the user can scroll it out of view, but it will reappear (slide in) after a moment?

What – if anything – can we do to have the toolbar behave in similar ways on iOS13 and iOS 11 as it does on iOS 12? [1]

For iOS 13 it’s probably possible, but I (or someone else) would have to experiment with it for a few hours to find out.

For iOS 11 we’d first have to get a device running that version to experiment with.

For iOS 13 it’s probably possible, but I (or someone else) would have to experiment with it for a few hours to find out.

Ok, let's do that. I'd like for us to prioritize looking into this.

I'm moving this to "Incoming" on the workboard assuming it doesn't make sense to create another task even tho this one already has a patch on it.

If it'd be more helpful for me to make another task and leave this in "Code review" please let me know.

For iOS 11 we’d first have to get a device running that version to experiment with.

Understood. Let's leave this be for now.

Decided:

  • Automatically close keyboard so toolbar is within view

Details:

  • Limited to mobile, iOS 13 and tbd scrolling behavior (see "Open questions" below)
  • When a contributor starts to scroll by dragging a selection handle, keyboard should not be hidden

Open questions:

  • What scrolling behavior should trigger the keyboard being closed?

If the above doesn't work...:

  • We'll try to get to the root cause of the issue and come up with another fix
ppelberg assigned this task to matmarex.Oct 30 2019, 7:55 PM
ppelberg added a project: Editing Design.

I feel that making it work as desired with the selection handles will be difficult, so before we try diving into that, I wanted to spend some time investigating the alternative approach first ("get to the root cause of the issue and come up with another fix").

Debugging this is a bit of a pain since the library I use to connect iOS devices to a browser debugger doesn't yet have a Windows version of the release supporting iOS 13 (https://github.com/google/ios-webkit-debug-proxy/issues/314#issuecomment-539979945). That comment advises building it myself, which I tried, and found out that I first need various dependencies, no idea how long it would take to track them all down. So I had to make do with trial and error and displaying values on the screen, like pioneers used to.

Anyway, I changed some things, and it helped a bit (with this patch, the toolbar does eventually show up in the right place), but didn't fix the problem entirely (it doesn't animate like it should and sometimes flashes in random places on the screen).

Change 547365 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Improve toolbar scrolling behavior on iOS 13

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

This is much better!

This is much better!

+1 – nice, Bartosz [1].

...I'm thinking it would be nice to use it first-hand before putting the patch on Tuesday's train.


  1. The "pioneer" comment was nice.
Esanders added a comment.EditedOct 31 2019, 3:18 PM

Yes, I put this on the prototype for testing. I found an issue when scrolling a long way (~2-3 pages worth?). Fixed

Anyway, I changed some things, and it helped a bit (with this patch, the toolbar does eventually show up in the right place), but didn't fix the problem entirely (it doesn't animate like it should and sometimes flashes in random places on the screen).

After some more fiddling I've found a way to mostly eliminate the problems, I'm still unhappy about this but I think we can live with it:

Outstanding issues are:

  • The scroll bar will flash when the toolbar adjusts itself (and there's a very short period of 50ms where you can't start scrolling again when this happens)
  • The toolbar occasionally flickers crazily while scrolling, this is visible at the very end of this recording

I don't know if we can do anything about these, but I think we can just leave them be, eh.

Also, someone should test on iOS 12 and verify that the changes I made here don't cause anything crazy to happen.

Did a brief test on iOS 12.4 on crossbrowsertesting, and it seemed to work:

ppelberg added a subscriber: iamjessklein.EditedOct 31 2019, 9:12 PM

Yes, I put this on the prototype for testing...

Cool. Thanks, Ed.

Even with the issues mentioned in T233470#5624402, I think this is a nice improvement. Good work, @matmarex.

@iamjessklein, what do you think? Do you think the way the patch behaves is an improvement over what is existing?

As for QA, @Ryasmeen, do you have access to a device running iOS 12 you're able to use to QA this? Ed did an initial pass; ideally, we have a device we can test this on first-hand.

Provided @iamjessklein and @Ryasmeen don't catch anything in the course of their reviews, I think we should release this.


A potentially related question: in the course of testing the toolbar patch, I noticed – what I think is – a pattern where the edit card will not appear when selecting a link while the keyboard is open [1]...

  • 1. Are y'all able to reproduce this?
  • 2. If yes, might this Edit Card issue be related to this toolbar issue? If not, I will file a separate task.

  1. See the the 9 seconds beginning at 0:07 in this video: https://youtu.be/fGonV99bR60?t=7
ppelberg added a project: Editing QA.EditedOct 31 2019, 9:38 PM

As for QA, @Ryasmeen, do you have access to a device running iOS 12 you're able to use to QA this? Ed did an initial pass; ideally, we have a device we can test this on first-hand.

Just chatted with @Ryasmeen, she thinks she'll be able to test this patch on both iOS 12 and iOS 13. Tho, I'll leave Rummana to confirm.

I tested it this morning:

  • 👍 the patch looks good
  • 🤯 I wasn't able to reproduce the issue that @ppelberg identified
ppelberg reassigned this task from matmarex to Ryasmeen.Nov 4 2019, 4:43 PM

I tested it this morning:

  • 👍 the patch looks good
  • 🤯 I wasn't able to reproduce the issue that @ppelberg identified

Ok, great. Thank you for reviewing, @iamjessklein. Noted RE not being able to reproduce the issue mentioned in T233470#5624784.

Alright @Ryasmeen, this is ready for your review.

(Note that this is only available on the prototype server, https://visualeditor-prototype.wmflabs.org/, as we haven't merged the patch yet)

Ryasmeen added a comment.EditedNov 5 2019, 1:14 AM

As for QA, @Ryasmeen, do you have access to a device running iOS 12 you're able to use to QA this? Ed did an initial pass; ideally, we have a device we can test this on first-hand.

Just chatted with @Ryasmeen, she thinks she'll be able to test this patch on both iOS 12 and iOS 13. Tho, I'll leave Rummana to confirm.

@ppelberg: So the device I thought was running iOS 12, is actually running iOS 11. But the good news is, it seems to be working perfectly on iOS 11 as well, just like how it behaves on iOS 13: it scrolls out of view when scrolling down the page, but it reappears (slides in) after a moment. I was under the impression (from reading @matmarex's comment) that it won't work that way on iOS 11. So that's good.

Now the question is, do you want me to test it on iOS 12 as well, in that case I will upgrade the iOS version on that device and test it again. Otherwise maybe it's better for us to have a device that's running iOS 11?

Also, I am noticing the same weird flickering that @matmarex mentioned, but that's already a known issue.

Lastly, the issue of edit card occasionally not appearing has happened to me as well, but I think it's an existing transient issue, which I couldn't reproduce reliably. I can't remember/find if there is a ticket for this, maybe we should file one.

I've merged this patch so it will go out on Thursday.

Change 547365 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Improve toolbar scrolling behavior on iOS 13

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

Thank you for going through this, Rummana. A few comments in-line, below...

@ppelberg: So the device I thought was running iOS 12, is actually running iOS 11. But the good news is, it seems to be working perfectly on iOS 11 as well, just like how it behaves on iOS 13: it scrolls out of view when scrolling down the page, but it reappears (slides in) after a moment. I was under the impression (from reading @matmarex's comment) that it won't work that way on iOS 11. So that's good.

Patch works well on iOS 11. Great.

Now the question is, do you want me to test it on iOS 12 as well, in that case I will upgrade the iOS version on that device and test it again. Otherwise maybe it's better for us to have a device that's running iOS 11?

Yes, can you please test this patch on iOS 12 as well, and in the process, upgrade the iOS version on the device you had been using to test patches on iOS 11?

Thinking: 40% of Wikipedia's iOS traffic comes from devices running iOS 12 [vs. ~4.0% from iOS devices running iOS 11]. Thus, I think it's important for us to prioritize testing on iOS 12 above testing on iOS 11. | Source

Also, I am noticing the same weird flickering that @matmarex mentioned, but that's already a known issue.

Lastly, the issue of edit card occasionally not appearing has happened to me as well, but I think it's an existing transient issue, which I couldn't reproduce reliably. I can't remember/find if there is a ticket for this, maybe we should file one.

This is helpful to know. I've created this ticket as a placeholder: T237475

Although, tho please boldly close this newly created ticket as a duplicate if you happen upon a ticket that's already been filed.

@ppelberg: So the device I thought was running iOS 12, is actually running iOS 11. But the good news is, it seems to be working perfectly on iOS 11 as well, just like how it behaves on iOS 13: it scrolls out of view when scrolling down the page, but it reappears (slides in) after a moment. I was under the impression (from reading @matmarex's comment) that it won't work that way on iOS 11. So that's good.

This is a bit surprising, but I guess it's possible. Glad I don't have to think about that one again. :)

Change 526235 abandoned by Bartosz Dziewoński:
MobileArticleTarget: Limit iOS fixed toolbar workarounds to iOS 12 only

Reason:
Workarounds were fixed properly by https://gerrit.wikimedia.org/r/547365

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

JTannerWMF moved this task from Inbox to High Priority on the Editing QA board.Feb 6 2020, 11:35 PM
JTannerWMF added a subscriber: JTannerWMF.

Hey @Ryasmeen, please move this to Peter's column if it is done with QA, this so we can close the parent task.

matmarex moved this task from To Triage to Triaged on the VisualEditor board.Feb 12 2020, 5:22 PM
ppelberg closed this task as Resolved.Feb 15 2020, 2:03 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptFeb 15 2020, 2:03 AM