Page MenuHomePhabricator

Code block dialog should not close on Escape if there are unsaved changes
Open, In Progress, Needs TriagePublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Edit a page in VisualEditor
  • Insert a new code block (Insert -> Code block or type <syntax)
  • The Code block dialog opens
  • Type some code into the dialog
  • Accidentally press the Escape key

What happens?: The dialog closes. The code I typed is lost.

What should have happened instead?: The dialog should warn that I'm about to lose what I typed.

Software version (skip for WMF-hosted wikis like Wikipedia): WMF production

Other information (browser name/version, screenshots, etc.):

Event Timeline

It makes sense that it's happening, insofar as pressing the X on the dialog does the same and escape is just a shortcut to that. It's also a pretty general behavior -- I think every dialog behaves that way in VE, not just the code block one?

...that said, I'd agree that a confirmation step for discarding changes might be a sensible general addition to these things.

There's one dialog with a confirmation step, the template dialog (added by WMDE as a part of their template work). It might make sense to add one to this dialog as well (but not necessarily every dialog, as many of them are quite simple). In particular, because in the code dialog, Escape has other functions (it cancels the code editor autocompletion), so it's easy to press by accident.

Change #1020294 had a related patch set uploaded (by Zoe; author: Zoe):

[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Confirms before closing code block dialog if the user has pending changes

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

I have a patch ready for this specific plugin. However, it made sense to move this change over to the VE codebase such that any plugin that creates a dialog can benefit from the check for unsaved changes. I'll be submitting that shortly for feedback.

Change #1025369 had a related patch set uploaded (by Zoe; author: Zoe):

[mediawiki/extensions/VisualEditor@master] Confirm abort if saving would make a change to the underlying document

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

Change #1025384 had a related patch set uploaded (by Zoe; author: Zoe):

[mediawiki/extensions/VisualEditor@master] Removed the unused 'whitespace' field

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

Change #1025390 had a related patch set uploaded (by Zoe; author: Zoe):

[mediawiki/extensions/VisualEditor@master] Rename isModified to isSaveable and deprecate the old name.

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

Change #1025384 abandoned by Zoe:

[mediawiki/extensions/VisualEditor@master] Removed the unused 'whitespace' field

Reason:

Conflicts with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/620362 which renders this unnecessary

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

Change #1020294 abandoned by Zoe:

[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Confirms before closing code block dialog if the user has pending changes

Reason:

Moved the change into VE as it's generic https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/1025369

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

Esanders moved this task from Incoming to QA on the Editing-team (Kanban Board) board.
Esanders added a project: Editing QA.
Esanders moved this task from Inbox to Low Priority on the Editing QA board.

Change #1025369 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Confirm abort if saving would make a change to the underlying document

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

Change #1025390 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Rename isModified to isSaveable and deprecate the old name.

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

zoe changed the task status from Open to In Progress.Wed, May 1, 1:47 PM
EAkinloose subscribed.

Thanks for catching this.

{F50043827}