Page MenuHomePhabricator

MobileFrontend editor loads on top of WikiEditor when following a link to action=edit
Closed, ResolvedPublic

Description

MobileFrontend editor loads on top of WikiEditor when following a link to action=edit, e.g. https://en.m.wikipedia.org/w/index.php?title=The_Fighting_Temeraire&action=edit

This is bad because:

Event Timeline

Change 904565 had a related patch set uploaded (by Bartosz Dziewoński; author: DLynch):

[mediawiki/extensions/MobileFrontend@master] Don't show a flicker of WikiEditor before displaying the MF editor

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

Change 906648 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/WikiEditor@master] Remove hacks that avoid duplicate event logging with MobileFrontend

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

Change 904565 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Don't show a flicker of WikiEditor before displaying the MF editor

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

@DLynch There is an edge case where the WikiEditor is still used even in MobileFrontend. When you go to the history of a page, and undo an edit, it uses the WikiEditor instead of the MobileFrontend editor. The patch breaks that, in that the placeholder message is shown without an editor appearing, and you have to specifically click the "reload this page" link.

I created a patchdemo wiki from your patch, so you can test it by clicking "Undo" in the following link. Before you do so, you have to enable the MobileFrontend (obviously), and also enable the "Advanced mode" within MobileFrontend (if you don't, the "Undo" link won't appear). https://patchdemo.wmflabs.org/wikis/1441c38ec4/w/index.php?title=New_York_Yankees&action=history

@jhsoby I'm assuming that the reason we don't want to make the MF editor take over that as well would be that it'd lose the visible diff above the editor, so I'll add an exception. I think this also wound up suppressing action=edit&section=new which is similar.

This comment was removed by DLynch.

the MF editor

Enough is enough! I've had it with these MobileFrontend snakes on this MobileFrontend plane!

(Sorry, just had to.)

But yeah, exceptions for those cases sounds good. I'm not aware of any other cases, but maybe there's some way to deduce if there are any more from the MF code?

Change 906760 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/VisualEditor@master] Don't hook CustomEditor for MobileFrontend requests

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

Change 906761 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/MobileFrontend@master] Don't suppress WikiEditor in some edge cases

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

Change 906761 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Don't suppress WikiEditor in some edge cases

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

Change 906760 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't hook CustomEditor for MobileFrontend requests

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

Change 906648 merged by jenkins-bot:

[mediawiki/extensions/WikiEditor@master] Remove hacks that avoid duplicate event logging with MobileFrontend

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

Likely caused by this change:

The change seems to have been catastrophic for those who do counter-vandalism or edit with no JS on mobile. I suggest reverting and backporting.

Change 909328 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/MobileFrontend@master] Clean up mobile editor initialization checks

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

matmarex removed a project: Editing QA.

I submitted patches for those issues (which we'll hopefully backport today), and the patch above is a refactoring to avoid similar problems in the future.

Change 909328 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Clean up mobile editor initialization checks

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

Heads up to QA: we've now wound up touching literally-everything about when the MobileFrontend editor initializes itself, so anything at all you can think of to test where the mobile editor should / should-not be counts against this ticket.

Ryasmeen changed the task status from Open to In Progress.May 11 2023, 5:09 PM
Ryasmeen claimed this task.
Ryasmeen added a subscriber: DLynch.

A few observations:

  1. Clicking on a link with action=edit now opens the MobileFrontend editor without showing the WikiEditor before loading it. However, during the transition, another state/page briefly appears where it shows the placeholder message with the "reload this page" link and then it loads the MobileFrontend editor.
  1. If you click on a link with action=edit as a logged-out user, it will show you the page with the warning message for logged-out users. But, when you click on the 'X' button on the top left, it shows you that same intermediate state/page thats appears for the issue described above and it stays there without navigating to the Read Mode or the article page. There are also no tabs on the top of the page to get out of this state by navigating away.

Screenshot 2023-05-11 at 11.17.21 PM.png (776×2 px, 109 KB)

Screenshot 2023-05-11 at 11.17.30 PM.png (758×1 px, 99 KB)

Ryasmeen removed a project: Editing QA.
Ryasmeen subscribed.

1 is the correct new-behavior.

2 is something we should adjust, though. We cope correctly with the same pattern on desktop-VE (by navigating to the read view), so it probably just needs some special case in the dismissal path.

Change 931501 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/MobileFrontend@master] When on a hijacked action=edit page, reload page when leaving the editor

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

Change 934342 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/VisualEditor@master] MobileArticleTarget teardown behavior is upstreamed to MobileFrontend

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

Change 931501 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] When on a hijacked action=edit page, reload page when leaving the editor

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

Change 934342 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] MobileArticleTarget teardown behavior is upstreamed to MobileFrontend

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