Last modified: 2014-09-23 23:53:38 UTC
The "edit warning" feature of the Vector skin does not pop up a warning when the user uses the SimpleSearch box. Use case: 1. Edit any article. 2. Make a change in the edit box but do not save. 3. (Optional) Click the "History" link: confirm that you get a proper edit warning. Cancel it. 4. Remaining on the edit page, type text into the search box and press Enter. 5. You leave the edit page without a warning, even though the text is modified. You have now lost your unsaved work.
By "edit warning" do you mean the confirmation dialog for navigating away from the page?
Yes, that's right. I used the phrase "edit warning" because that's how it's referred to in the code: Vector/modules/ext.vector.editWarning.js, etc.
Created attachment 8846 [details] Proposed patch The warning is shown by the beforeunload handler, but this handler is removed when any form is submitted though it should be removed only if the editform is submitted. The patch fixes this. Additionally I used mw.config.get for the global variable and changed some other stuff (mediaWiki -> mw, size() -> length, === for comparison to 0).
Clicking "stay on page" on the dialog that pops up now after using the enter key in the search field ... doesn't stop you from leaving the page. Tested on FF5. Works as expected otherwise.
Created attachment 8882 [details] Proposed patch - part 2 Found the culprit in jquery.suggestions.js. When you press enter, the javascript tries to submit the form, so editwarning triggers correctly. Editwarning unbinds the onbeforeunload handler (caching reasons in Firefox). Then the default reaction to the enter key is done: the form is submitted again (probably browser specific). This patch simply prevents this default. Both patches together should solve the problem thoroughly.
The proposed 2 patches work for me in Firefox and Chrome. In Internet Explorer 8, however, the edit warning is not appearing at all (in Vector skin) at this point.
How about making this part of 1.19? It can lead to data loss (e.g., the user does a search while on the edit page, and unwittingly loses whatever she was working on).
My earlier comment #6 about Internet Explorer should be ignored. The edit warning is indeed appearing in IE.
Michael, sorry for the wait; there's been a bit of a delay in the review of patches here. As we prepare to get a new version out, we're in a "code slush" during which we concentrate on reviewing code that has already been committed to our source code repository (see http://thread.gmane.org/gmane.science.linguistics.wikipedia.technical/57950 for more details). I hope to get a reviewer to check your contribution soon. In the interim, if you want, you could mark your old patches obsolete and simply combine them into one patch and attach it, which would be slightly easier for us to test and apply. Thanks for the patch!
Dan, have you gotten a Developer access account yet? https://www.mediawiki.org/wiki/Developer_access
Thanks Sumana. I have not requested developer access due to lack of time for any development. Maybe in the future.
Thank you for the patch Michael. I've applied part of this fix as the code base has changed a bit since you posted. Seems to be working now in FF, Chrome, and IE. Status of the change can be viewed here. https://gerrit.wikimedia.org/r/#change,5734
(Quote from comment #4) > Clicking "stay on page" on the dialog that pops up now after using the enter > key in the search field > > ... doesn't stop you from leaving the page. Tested on FF5. Still reproducible, please also apply the second part of the proposed patch. (Due to bug 37479 you have to press enter in an empty search field.)
(In reply to comment #13) > (Quote from comment #4) > > Clicking "stay on page" on the dialog that pops up now after using the enter > > key in the search field > > > > ... doesn't stop you from leaving the page. Tested on FF5. > > Still reproducible, please also apply the second part of the proposed patch. > (Due to bug 37479 you have to press enter in an empty search field.) When you tested it it may not have been deployed yet. Merging is not the same as deploying (every 2 weeks, the latest merge state in master is deployed). Can you try again now?
(In reply to comment #14) > When you tested it it may not have been deployed yet. Merging is not the same > as deploying (every 2 weeks, the latest merge state in master is deployed). > > Can you try again now? I tested it on mw.org, where 1.20wmf5 was already deployed at that time. And yes, I can still reproduce it on any WMF wiki. 1. Enable EditWarning in your preferences. 2. Open a page in edit mode. 3. Type something to change the text. 4. Press <Enter> in the empty search field. 5. A message will pop up, asking you if you really want to leave the page (as expected), click "Stay on page" 6. Despite your choice you will leave the page, and go to Special:Search. Reproducible at least with Firefox (if I remember correctly, FF submits a form when you press enter; in SimpleSearch there is a missing event.preventDefault(), so the search form is submitted twice, while edit warning only catches it once).
I just reproduced this on test.wikipedia.org, which is running MW core from a day ago and Vector code from Sept. 30th. In fact the case is slightly worse now than it was in June. (A regression, but not necessarily a regression within the last 2 weeks so I'm not keywording it as a regression.) We once more have the situation as described in the original bug report: 1. Enable EditWarning in your preferences (Special:Preferences#mw-prefsection-editing, "Warn me when I leave an edit page with unsaved changes"). 2. Open a page in edit mode. 3. Type something to change the text. 4. Press <Enter> in the empty search field. 5. The "do you really want to leave?" message DOES NOT pop up; you will leave the page, and go to Special:Search. This will lead to data loss by people who aren't quick-thinking enough to immediately hit the Back button. Thus increasing the priority.
I can only reproduce this original bug in Firefox. I have submitted Michael's second patch in https://gerrit.wikimedia.org/r/#/c/36123/ Which, I have testing and seems to fix this bug. This change is not yet merged, though I have pulled it to the micro-design instance for testing. http://micro-design.wmflabs.org/index.php?title=Main_Page&action=edit
This is now fixed in Vector. Thanks for the fix, Rob! Problem still happens in Monobook in Firefox. Also there's an edge case where it still happens in Vector, as Rob and I just discovered: 1. Enable EditWarning in your preferences (Special:Preferences#mw-prefsection-editing, "Warn me when I leave an edit page with unsaved changes"). 2. Open a page in edit mode. 3. Type something to change the text. 4. Press <Enter> in the empty search field. 5. The "do you really want to leave?" message pops up. Hit Leave Page. 6. On the new page, hit Back. 7. Now click in the search field and hit <Enter>. Even though the article text has changed and not been saved, the "wait, really?" dialog box does NOT pop up, defying the promise of the edit warning preference! This is because the page is loaded and we don't detect a text change. Now this is lower-priority, I think.
Change-Id: I3cb40f70bf332e4aa53cd656b1c847b5f4637360
(In reply to comment #18) > This is now fixed in Vector. Thanks for the fix, Rob! > > Problem still happens in Monobook in Firefox. > I can no longer reproduce the bug in Vector, Monobook both in both Chrome and Firefox. > Also there's an edge case where it still happens in Vector, as Rob and I just > discovered: > > 1. Enable EditWarning in your preferences > (Special:Preferences#mw-prefsection-editing, "Warn me when I leave an edit > page > with unsaved changes"). > 2. Open a page in edit mode. > 3. Type something to change the text. > 4. Press <Enter> in the empty search field. > 5. The "do you really want to leave?" message pops up. Hit Leave Page. > 6. On the new page, hit Back. > 7. Now click in the search field and hit <Enter>. Even though the article > text has changed and not been saved, the "wait, really?" dialog box does > NOT pop up, > defying the promise of the edit warning preference! This is because the page > is loaded and we don't detect a text change. Confirmed that this bug is still happening in Chrome and Firefox in both Vector and Monobook. However the patch submitted on this bug (in Gerrit as I3cb40f70bf) does NOT fix it (no visible change in behaviour).
I cant reproduce the original bug but when i press enter in an empty search box while editing a page, then even if i choose stay on page, it still go to the search page discarding all the changes. Reproduced on FireFox
Confirming comment 21 with Firefox 25.0 with settings described in comment 18.
