Last modified: 2013-03-18 21:18:48 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T44939, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 42939 - VisualEditor: Save button should be disabled if dm state is identical to page when it was loaded
VisualEditor: Save button should be disabled if dm state is identical to page...
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
MediaWiki integration (Other open bugs)
unspecified
All All
: Normal normal
: VE-deploy-2013-03-18
Assigned To: Ed Sanders
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-11 04:18 UTC by James Forrester
Modified: 2013-03-18 21:18 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description James Forrester 2012-12-11 04:18:27 UTC
Right now, the Save button starts disabled, but when the transaction depth is non-zero it's enabled. This means that two actions that reverse each other (rather than being formally undone) let the user make a 'null edit', which MW then silently accepts.

Ideally, the Save/Create button should be disabled if dm state is identical to page when it was loaded (except for the special-case of oldid pages, where it should always be enabled). Roan suggests a string comparison of the current and original HTML is the least-bad option, given the lack of hash functions in JavaScript.
Comment 1 Ed Sanders 2013-03-14 09:52:23 UTC
Right now the Save button becomes enabled after the first transaction is applied and never goes back to disabled, even if the first transaction is undone. While this isn't ideal, doing a full HTML comparison on pretty much every keystroke may be a bit of an unnecessary overhead. Even doing an transaction stack comparison is a little unnecessary.

The real problem here is the creation of null edits and that only needs to be addressed when the save button is pressed. This could trigger an alert ("You haven't made any changes") and reset the button to disabled. We should also run this check in the window.onbeforeunload handler so people can leave the page nag-free if they undo their edits.
Comment 2 James Forrester 2013-03-14 17:07:10 UTC
Agree on the window.onbeforeunload.

Not totally enamoured with the idea of a modal alert telling users off for behaviour that is currently silently failing (in core wikitext editor, if you make a null save it'll look like it saved, but no new entry in the history log).

Possibly we could over-ride the edit-success box to say "You made no changes so nothing was saved" or something similar? We should probably talk to E3 to see if this is something they'd want to see happen in core anyway...
Comment 3 Ed Sanders 2013-03-14 17:19:02 UTC
After some more investigation is appears that comparing html might not do the trick. Doing something trivial like typing and deleting may not leave the html in it's original state. We currently have change markers (although these are queued up for deletion), but also hidden things like pawn characters and maybe other things in the future.

We can still implement the transaction stack check which we don't have at the moment, but telling you if there is a non-zero diff may have to be a job for Parsoid.
Comment 4 Ed Sanders 2013-03-15 15:59:02 UTC
Fixed the zero-history case, and the onbeforeunload issue: https://gerrit.wikimedia.org/r/#/c/53988/
Comment 5 Ed Sanders 2013-03-15 16:00:13 UTC
I'd suggest that once that change has been merged this bug can be closed.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links