Page MenuHomePhabricator
Create Task
Maniphest T334513

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

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.):

Details

SubjectRepoBranchLines +/-
Rename isModified to isSaveable and deprecate the old name.mediawiki/extensions/VisualEditormaster+10 -4
Confirm abort if saving would make a change to the underlying documentmediawiki/extensions/VisualEditormaster+82 -1
Confirms before closing code block dialog if the user has pending changesmediawiki/extensions/SyntaxHighlight_GeSHimaster+61 -2
Removed the unused 'whitespace' fieldmediawiki/extensions/VisualEditormaster+0 -3
Customize query in gerrit

Event Timeline

Catrope created this task.Apr 11 2023, 7:34 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 11 2023, 7:34 PM
DLynch subscribed.Apr 12 2023, 12:19 AM

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.

matmarex moved this task from To Triage to Triaged on the VisualEditor board.Apr 20 2023, 2:23 PM
matmarex subscribed.Apr 20 2023, 2:26 PM

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.

zoe subscribed.Wed, Apr 10, 9:51 AM
gerritbot added a comment.Tue, Apr 16, 5:20 PM

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

gerritbot added a project: Patch-For-Review.Tue, Apr 16, 5:20 PM
PatchDemoBot added a comment.Wed, Apr 17, 12:57 PM

Test wiki created on Patch demo by ESanders (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/3fbaeb58ab/wiki/Syntax

zoe claimed this task.Thu, Apr 18, 9:32 AM
zoe added a comment.Mon, Apr 29, 10:52 AM

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.

gerritbot added a comment.Mon, Apr 29, 2:01 PM

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

gerritbot added a comment.Mon, Apr 29, 2:58 PM

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

gerritbot added a comment.Mon, Apr 29, 3:18 PM

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

gerritbot added a comment.Mon, Apr 29, 3:31 PM

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

gerritbot added a comment.Tue, Apr 30, 9:12 AM

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 added a project: Editing-team (Kanban Board).Tue, Apr 30, 3:59 PM
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.
gerritbot added a comment.Tue, Apr 30, 5:14 PM

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

gerritbot added a comment.Tue, Apr 30, 5:14 PM

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

Maintenance_bot removed a project: Patch-For-Review.Tue, Apr 30, 5:30 PM
ReleaseTaggerBot added a project: MW-1.43-notes (1.43.0-wmf.4; 2024-05-07).Tue, Apr 30, 6:00 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL