Page MenuHomePhabricator

TwoColConflict gives no warning if closing the tab or using browser back button
Closed, InvalidPublic3 Estimated Story Points

Description

If you edit an article and have changed anything and close the Browser-Window/Tab or click the "cancel"-button a warning that you will lose your edit will be displayed.

This should also be the case for TwoColConflict-Pages.

It seems that the "Cancel" button correctly prompts for unsaved changes, and after you've done that, closing the tab also correctly prompts. The bug only appears if you close the tab before clicking any of the controls.

Event Timeline

Restricted Application added a project: archived--TCB-Team. · View Herald TranscriptOct 16 2018, 11:47 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Note: maybe this is already the case but just not for the simulation-page!?

awight reopened this task as Open.Feb 14 2020, 9:54 AM
awight added a subscriber: awight.

I'm unmerging this bug, because I found it's still an outstanding issue, and both conceptually and functionally different from the other task.

Steps to reproduce:

  • Cause an edit conflict.
  • Do nothing at all on the conflict page.
  • Close the tab.

This will silently lose your edits. Reopening the page even with Visual Editor will not restore your work.

awight renamed this task from No warning if closing the tab or clicking cancle in TwoColConflict to No warning if closing the tab or clicking cancel in TwoColConflict.Feb 14 2020, 9:57 AM
awight updated the task description. (Show Details)
awight updated the task description. (Show Details)Feb 14 2020, 10:02 AM

I think this can be solved by setting the unsaved changes flag immediately upon entering the page.

awight renamed this task from No warning if closing the tab or clicking cancel in TwoColConflict to TwoColConflict gives no warning if closing the tab or using browser back button.Feb 14 2020, 10:22 AM
thiemowmde set the point value for this task to 3.
WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-02-19 board.

Just two first observations here: When you get to the conflict solving screen and you click anywhere on the page, then the "leave warning" will be "enabled" meaning that the warning is shown when you try to leave the page. This is also true for the case that you click any link on the page.

Manually triggering something like $( '#editform' ).trigger( 'change' ); directly after the page was loaded does not help.

Change 575206 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] [WIP] Confirm close in any case

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

We're currently triggering the "pending changes" handler in core[1]. It fires as soon as someone changed the text in the editor on the edit page. Technically we do not have the editor that the handler watches but since it's missing on our interface and the handler is still loaded it fires in any case(*) although nothing changed on our interface yet.

Users can decide if they want to get that "pending changes" warning via their user preferences.
Editing > Editor > Warn me when I leave an edit page with unsaved changes

(*)NOTE: We can not trigger a warning if the user is leaving the page before having at least interacted once with it. ( e.g. by clicking somewhere )

What we ( at least ) could do here: Add our own "pending changes" handler that does not rely on the "accidentally" triggered handler from the normal edit process. The question is, if we would want to allow disabling it via the same setting in the user's preferences like the "normal" "pending changes" handler from the edit page.

[1] https://gerrit.wikimedia.org/g/mediawiki/core/+/cb97bd764a83a1edc5e98807b9d19913706cc02b/resources/src/mediawiki.action/mediawiki.action.edit.editWarning.js

I forgot to add the link why we cannot trigger a warning before the user interacted:

To combat unwanted pop-ups, browsers may not display prompts created in beforeunload event handlers unless the page has been interacted with, or may even not display them at all.

From https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event

Change 575471 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/core@master] Don't warn users of pending changes when there's no textbox

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

Change 575471 abandoned by WMDE-Fisch:
Don't warn users of pending changes when there's no textbox

Reason:
As mentioned in the patch. This whole idea was based on a wrong assumption. The code does exactly what is needed to enable the edit warning on the conflict solving case.

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

Change 575206 abandoned by WMDE-Fisch:
[WIP] Add handler for pending changes

Reason:
As mentioned in the patch in core. This whole idea was based on a wrong assumption. The code does exactly what is needed to enable the edit warning on the conflict solving case.

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

Change 575522 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/core@master] Improve documentation in editwarning script

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

Change 575522 merged by jenkins-bot:
[mediawiki/core@master] Improve documentation in editwarning script

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

awight added a comment.Mar 2 2020, 8:17 AM

Please document the conclusions on the ticket--it seems that the feature is impossible to implement at the moment so we should mark this as "declined" or "stalled"? I want this point to be visible, because it suggests we need to think about other mechanisms for protecting users against accidental loss of editing data. Maybe write a spike for further investigation?

thiemowmde added a subscriber: thiemowmde.

TL;DR: The description in the commit message describes a browser feature we can not change.

Modern browsers don't allow a tab to block itself from being closed as long as the user did not interacted with the tab. You can test this with the good old wikitext editor: First, make sure you have the "warn on close" setting enabled. Then open a random page on Wikipedia, click "edit source" and immediately close the tab. No warning.

When you do the same and click on a link inside the tab instead (e.g. go back to the main page), the warning will – in our case – always be shown. (Really always, because the assumption is that the conflict resolution screen always represents an unsaved edit, entirely independent from the selections made in the TwoColConflict interface.)

This makes this ticket a no-op. The "warn on close" feature works as expected, without any extra code in the TwoColConflict codebase. All patches here are abandoned.

awight added a comment.Mar 2 2020, 9:25 AM

You can test this with the good old wikitext editor

Thanks for the summary, it makes sense. The difference however is that no data is lost when a fresh wikitext editor is closed. In our case, data will be lost.

This makes this ticket a no-op. The "warn on close" feature works as expected, without any extra code in the TwoColConflict codebase. All patches here are abandoned.

I disagree with this "no-op" resolution, the user story is not satisfied. I think we're all on the same page, that the initial criterion "a warning that you will lose your edit will be displayed" is not going to happen. My thought is that we need to make it very explicit that we're failing to do this. Honestly, the problem is that we don't have much experience failing to complete tickets, so there isn't an obvious workflow for it :-)

awight added a comment.Mar 2 2020, 9:38 AM

We can change this to a investigation ticket and mark as completed, I have no strong opinions about how we do the bookkeeping. The only thing I really want to keep track of is that we are still losing data, so there should be a follow-up task to plug the hole using some other mechanism.

Sorry, I don't get what you are proposing. First, we can clearly argue that the "issue" is not an issue, but an intentional browser feature. Second, the same happens in the old conflict interface.

thiemowmde closed this task as Invalid.Mar 3 2020, 12:42 PM