Page MenuHomePhabricator

Make `unload handlers` work for contributors using NWE
Closed, ResolvedPublic

Description

Actual behavior

  1. Enable the "New wikitext mode" as a Beta Feature
  2. Go to: https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Cats
  3. Click "Reply"
  4. Start writing a comment
  5. Click "Edit source"

⚠️ 6. The full page editor is loaded; comment draft is lost

Expected behavior

  1. Enable the "New wikitext mode" as a Beta Feature
  2. Go to: https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Cats
  3. Click "Reply"
  4. Start writing a comment
  5. Click "Edit source"

6. The browser displays a warning message [1] alerting contributors their draft comment will be discarded if they proceed


  1. Browser warning message: "Leave site? / Changes you made may not be saved."

Additional context: T240259#5873772 + T240259#5872964

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 12 2020, 12:37 AM
ppelberg updated the task description. (Show Details)Feb 12 2020, 12:40 AM
ppelberg updated the task description. (Show Details)Feb 12 2020, 12:49 AM

A quick fix would be to disable the edit tabs (or at least the affected ones) while a reply widget is open. This would be similar to what we are doing with disabling other reply links.

n.b. we would need to take care to disable the access keys too

Change 579049 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Simulate a 'beforeunload' event before loading the editor

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

Change 579049 abandoned by Bartosz Dziewoński:
Simulate a 'beforeunload' event before loading the editor

Reason:
Probably not a good idea to do it this way.

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

In planning today, @DLynch shared he will disable the link.

Change 588935 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/DiscussionTools@master] Disable VisualEditor page takeovers while a reply widget is open

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

Change 588935 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/DiscussionTools@master] Disable VisualEditor page takeovers while a reply widget is open

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

@DLynch, two questions about https://gerrit.wikimedia.org/r/588935:

  1. Does the patch disable all "edit" links on the page? E.g. section edit links as well as those atop the talk page.
  2. What happens in the following scenario: Someone opens the Reply widget and clicks an "edit" link elsewhere on the page without having entered any text into the Reply widget?

...I tried to test out the patch myself [1], but I have not yet been able to get it to work.


  1. http://patchdemo.wmflabs.org/wikis/8439cb46e0cc9a53333e3d778115e450/w/index.php/Talk:Main_Page

Yes, section edit links are handled as well.

To answer your second question... none of the links are actually disabled fully; instead the page-takeover behavior is disabled while a reply widget is open. This means that when you click these links they behave like any other link on the page. If you had the widget open without any text entered, that would make the edit link just be followed as a full page-load, without the page-takeover.

...I tried to test out the patch myself [1], but I have not yet been able to get it to work.

It's a pain to test out with patchdemo because it's somewhat wiki-config-dependent. It requires that you be on a wiki that's set to use NWE as its default... or to create an account and adjust your settings so that you're using NWE.

We talked about it in a meeting, and @Esanders supports the idea of making a generic hook (via mw.hook or a custom jQuery event or whatever) which VisualEditor/NWE can fire off to tell the page it's about to be taken over. This could then be registered by DiscussionTools (and other extensions like Thanks) so it can try to do a teardown there.

This is more complicated initially, but does have the benefit that any random extension/gadget that wants to clean itself up would be able to cleanly do so, without us having to add knowledge of other systems into DiscussionTools specifically.

I am not really convinced that the more complicated approach is going to be worth it. Can we go with the simple one now, and come back to this if anyone ever requests any random extension/gadget to trigger our unload events? It seems to me that the same problem with "fake unloading" would occur in VE, and I don't remember anyone ever complaining about it, even though it has existed for years now.

Change 588935 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Disable VisualEditor page takeovers while a reply widget is open

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

matmarex edited projects, added Editing QA; removed Patch-For-Review.
Ryasmeen edited projects, added Verified; removed Editing QA.May 26 2020, 8:29 PM
ppelberg closed this task as Resolved.Jun 5 2020, 1:33 AM