No timeout recovery when switching between visual and source editor
Open, Needs TriagePublicBUG REPORT
Actions

Assigned To

Authored By

	SUM1
	Apr 24 2022, 3:41 PM

Description

Steps to reproduce:

Go on any article on a Wikimedia project.
Click "Edit" or "Edit source".
Enter an internet condition that causes timeouts.
Switch to the other editor (source or visual).

What happens?:
The following popup shows.

Then, if you click "Cancel", VisualEditor fails to recover (as has been implemented in other scenarios such as clicking "Publish changes"). Instead, it gets hung on the loading bar,

and all your changes made so far are now inaccessible.

What should have happened instead?:
It should recover and allow the user to continue editing in the mode they were previously in.

Unrelated side note: in the other scenario of clicking "Publish changes" in a timeout condition, it would be preferable if the button didn't arbitrarily grey out after the timeout, forcing the user to exit out of the box and re-open it.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc.:
Windows 7
Chrome 99.0.4844.51

Details

Subject	Repo	Branch	Lines +/-
Use surface's cancel command to close the editor	mediawiki/extensions/VisualEditor	master	+13 -46
Remove redundant error message used sometimes when switching	mediawiki/extensions/VisualEditor	master	+3 -12
Always prevent default on <esc> while the ProgressDialog is open	VisualEditor/VisualEditor	master	+17 -0
ArticleTarget: Ensure dataPromise rejects when switching fails	mediawiki/extensions/VisualEditor	master	+13 -18

Customize query in gerrit

Related Objects

Mentioned In: T311467: [wmf.17-mobile] "Suggestions" label has no padding
Mentioned Here: T310695: Pressing Escape when the focus is outside of the editor (e.g. the sidebar language popup is open) closes VE

Event Timeline

SUM1 created this task.Apr 24 2022, 3:41 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 24 2022, 3:41 PM

Change 786331 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] ArticleTarget: Ensure dataPromise rejects when switching fails

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

gerritbot added a project: Patch-For-Review.Apr 26 2022, 3:33 PM

matmarex moved this task from To Triage to Triaged on the VisualEditor board.May 9 2022, 11:18 PM

matmarex added a project: Editing-team (Kanban Board).

Change 786331 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] ArticleTarget: Ensure dataPromise rejects when switching fails

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

Maintenance_bot removed a project: Patch-For-Review.May 10 2022, 12:30 AM

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.12; 2022-05-16).May 10 2022, 2:01 AM

@Esanders I'm not sure if that entirely resolves the problem?

I also noticed while testing that pressing Escape while the progress bar dialog is visible now closes the editor completely, instead of only closing that dialog. (may be caused by https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/786328)

matmarex assigned this task to • Esanders.May 23 2022, 5:09 PM

matmarex moved this task from Blocked / Needs More Work to Ready to Be Worked On on the Editing-team (Kanban Board) board.

ppelberg removed a project: Editing-team (Kanban Board).Jun 6 2022, 5:28 PM

Change 805789 had a related patch set uploaded (by Esanders; author: Esanders):

[VisualEditor/VisualEditor@master] Always prevent default on <esc> while the ProgressDialog is open

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

gerritbot added a project: Patch-For-Review.Jun 15 2022, 11:45 AM

The above patch ensures the ProcessDialog always captures <esc>, however I think the problem here is that DesktopArticleTarget is listening to escape at the HTMLDocument level. If the user presses escape while in the site search bar, it should not close the editor. We should move that binding to the surface like other commands.

Change 805810 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Use surface's cancel command to close the editor

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

Change 805810 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Use surface's cancel command to close the editor

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

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.17; 2022-06-20); removed MW-1.39-notes (1.39.0-wmf.12; 2022-05-16).Jun 15 2022, 8:00 PM

In T306763#7919334, @matmarex wrote:

I also noticed while testing that pressing Escape while the progress bar dialog is visible now closes the editor completely, instead of only closing that dialog. (may be caused by https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/786328)

In T306763#8005461, @Esanders wrote:

The above patch ensures the ProcessDialog always captures <esc>, however I think the problem here is that DesktopArticleTarget is listening to escape at the HTMLDocument level. If the user presses escape while in the site search bar, it should not close the editor. We should move that binding to the surface like other commands.

(Filed as T310695, and fixed)

Change 805789 abandoned by Bartosz Dziewoński:

[VisualEditor/VisualEditor@master] Always prevent default on <esc> while the ProgressDialog is open

Reason:

No longer needed

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

Maintenance_bot removed a project: Patch-For-Review.Jun 15 2022, 8:29 PM

Outstanding issues:

Something is messed up with the error message when switching with changes from VE:
(we should probably just get rid of the "Error loading data from server" prefix)

For comparison, when switching without changes, the error message isn't messed up: (and also has a "Try again" button)

When switching with changes from 2017WTE, the dialog isn't messed up and has a "Try again" button, but the button will actually switch while discarding your changes!

When switching from old WTE, the VE loading bar will appear and get stuck, with no error message:
(you can recover from this by pressing Escape)

matmarex attached a referenced file: F35245115: image.png. (Show Details)Jun 15 2022, 11:17 PM

matmarex attached a referenced file: F35245117: T306763a.webm. (Show Details)

matmarex attached a referenced file: F35245105: image.png. (Show Details)

matmarex attached a referenced file: F35245101: image.png. (Show Details)

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

[mediawiki/extensions/VisualEditor@master] Remove redundant error message used sometimes when switching

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

gerritbot added a project: Patch-For-Review.Jun 15 2022, 11:36 PM

Change 805919 merged by jenkins-bot: