Page MenuHomePhabricator
Create Task
Maniphest T351821

Edit Recovery interaction with browser data recovery
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To
Samwilson
Authored By
dom_walden
Nov 22 2023, 3:11 PM
Project Tags
Referenced Files
F42156859: T351821_ER_Local_Edge.webm
Feb 26 2024, 10:49 PM
F42156855: T351821_ER_Local_Safari.webm
Feb 26 2024, 10:49 PM
F42156851: T351821_ER_Local_FF.webm
Feb 26 2024, 10:49 PM
F42156848: T351821_ER_Beta_Edge.webm
Feb 26 2024, 10:49 PM
F42156845: T351821_ER_Beta_Safari.webm
Feb 26 2024, 10:49 PM
F42156837: T351821_ER_Beta_FF.webm
Feb 26 2024, 10:49 PM
Subscribers
Aklapper
dom_walden
GMikesell-WMF
Nardog
Tacsipacsi
TheresNoTime

Description

What is the problem?

I have noticed a few times when clicking "Discard changes" in the Edit Recovery popup that the wikitext remains unchanged, rather than reverting to the current revision of the article.

It does clear the data from IndexedDB, although a user might not realise that this has been done if the wikitext has not changed.

I believe it is because my browser is recovering the unsaved edits I made before Edit Recovery does. I am guessing that makes Edit Recovery think my unsaved edits are the original state of the article. Therefore, clicking "Discard changes" does not restore the correct thing.

Times I have seen this:

  • When doing a "soft" refresh on Firefox
  • When closing and reopening a tab (with ctrl+shift+t) on Firefox
  • Restoring tabs after closing and reopening Firefox
  • Sometimes when restoring tabs after I have simulated a crash on Firefox (by doing kill on the process)

I have yet to see this on Chromium. I don't know about other browsers.

I am not sure what we want to do in this case and whether there may be other interactions between Edit Recovery and the browser's native data recovery.

Steps to reproduce problem
  1. In Firefox, open a new tab and go to https://en.wikipedia.beta.wmflabs.org/w/index.php?title=PhonosTest-211&action=edit
  2. Make an edit
  3. Close the tab
  4. Restore the tab (ctrl+shift+t)
  5. In the "Changes recovered" popup, click "Discard changes"

Expected behaviour: Wikitext is restored to the state it was in step 1.
Observed behaviour: Wikitext is not changed. The edit you made in step 2 remains.

Environment

Browser: Firefox 115 and 119.
Wiki(s): https://en.wikipedia.beta.wmflabs.org MediaWiki 1.42.0-alpha (b2d4fd5) 14:40, 22 November 2023.
Editor: WikiEditor 0.5.4 (68da956) 07:21, 22 November 2023.

QA Results - Beta & Local

ACStatusDetails
1✅https://phabricator.wikimedia.org/T351821 here

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Edit Recovery: Use original data rather than re-reading currentmediawiki/coremaster+4 -3
Edit Recovery: Store better original form datamediawiki/coremaster+25 -4
Realtime Preview: also trigger on form.reset()mediawiki/extensions/WikiEditormaster+4 -0
Customize query in gerrit

Related Objects

Event Timeline

dom_walden created this task.Nov 22 2023, 3:11 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 22 2023, 3:11 PM
dom_walden mentioned this in T343089: Save edit recovery data on visibilitychange event.Nov 27 2023, 3:45 PM
gerritbot added a comment.Nov 28 2023, 5:00 AM

Change 977828 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] Edit Recovery: Reset form before saving original data

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

gerritbot added a project: Patch-For-Review.Nov 28 2023, 5:00 AM
Samwilson claimed this task.Nov 28 2023, 5:00 AM
Samwilson edited projects, added: Community-Tech (CommTech-Kanban); removed: Community-Tech.
Samwilson moved this task from Ready 🎬 to Review/Feedback 💬 on the Community-Tech (CommTech-Kanban) board.Nov 28 2023, 5:15 AM
Samwilson mentioned this in T351869: Soft refreshes can clear Edit Recovery data on Firefox.Nov 28 2023, 6:40 AM
Tacsipacsi subscribed.Nov 28 2023, 2:24 PM
Samwilson moved this task from Review/Feedback 💬 to In Development 💻 on the Community-Tech (CommTech-Kanban) board.Nov 29 2023, 11:36 AM
Samwilson moved this task from In Development 💻 to Review/Feedback 💬 on the Community-Tech (CommTech-Kanban) board.Dec 1 2023, 7:02 AM
gerritbot added a comment.Dec 7 2023, 5:27 AM

Change 980977 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/extensions/WikiEditor@master] Realtime Preview: also trigger on form.reset()

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

gerritbot added a comment.Dec 8 2023, 12:16 PM

Change 977828 merged by jenkins-bot:

[mediawiki/core@master] Edit Recovery: Store better original form data

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

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.9; 2023-12-12).Dec 8 2023, 1:00 PM
gerritbot added a comment.Dec 8 2023, 4:51 PM

Change 980977 merged by jenkins-bot:

[mediawiki/extensions/WikiEditor@master] Realtime Preview: also trigger on form.reset()

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

Maintenance_bot removed a project: Patch-For-Review.Dec 8 2023, 5:10 PM
Samwilson mentioned this in T353616: "Discard changes" does not discard changes for CodeEditor or CodeMirror.Jan 2 2024, 7:58 AM
Samwilson moved this task from Review/Feedback 💬 to In Development 💻 on the Community-Tech (CommTech-Kanban) board.Jan 3 2024, 11:44 AM
Samwilson moved this task from In Development 💻 to Review/Feedback 💬 on the Community-Tech (CommTech-Kanban) board.Jan 4 2024, 2:57 AM

This should now be fixed, as we've changed how we store the initial data of the form (for later restoration).

Part of this was to also change how we reset the form to the initial state, but as part of T353616 it looks like we need to change this back again. This shouldn't impact this bug, but I think this task should wait on QA until that one is sorted out. The outstanding change there is r/987029.

Nardog subscribed.Jan 4 2024, 8:41 AM
Samwilson moved this task from Review/Feedback 💬 to QA 🐛 on the Community-Tech (CommTech-Kanban) board.Jan 9 2024, 5:35 AM

This is ready for QA now.

dom_walden added a comment.Jan 12 2024, 10:44 AM

@Samwilson I am still able to reproduce this bug on Firefox on beta, but not locally.

Beta and production both have Cache-Control: private, which might be the difference.

Samwilson moved this task from QA 🐛 to In Development 💻 on the Community-Tech (CommTech-Kanban) board.Jan 17 2024, 11:35 AM
gerritbot added a comment.Jan 21 2024, 6:16 AM

Change 991951 had a related patch set uploaded (by Samwilson; author: Samwilson):

[mediawiki/core@master] Edit Recovery: Use original data rather than re-reading current

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

gerritbot added a project: Patch-For-Review.Jan 21 2024, 6:16 AM
Samwilson moved this task from In Development 💻 to Review/Feedback 💬 on the Community-Tech (CommTech-Kanban) board.Jan 21 2024, 6:22 AM

I'm able to reproduce this now. I'm not sure what was going wrong before but it's reliably wrong for me everywhere now.

But yep, this seems to be a complete oversight on my part in 36c3fc1992f7cd0d01bd33c1ad3f34bdb22cad0e when switching back from using HTMLForm.reset() to discard the form values (for T353616), when I added in code to re-read the current form values — even though there was already a variable called originalData right there waiting to be used.

I think you're right in that Firefox behaves differently when restoring cached form data (on re-opening windows) depending on the cache headers. But we want it to work the same always (which I think it now will… famous last words of course).

TheresNoTime subscribed.Jan 22 2024, 2:41 PM
TheresNoTime mentioned this in T355548: Edit Recovery deployment.Jan 22 2024, 3:28 PM
gerritbot added a comment.Jan 22 2024, 3:50 PM

Change 991951 merged by Samtar:

[mediawiki/core@master] Edit Recovery: Use original data rather than re-reading current

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

TheresNoTime moved this task from Review/Feedback 💬 to QA 🐛 on the Community-Tech (CommTech-Kanban) board.Jan 22 2024, 3:56 PM
Maintenance_bot removed a project: Patch-For-Review.Jan 22 2024, 4:31 PM
ReleaseTaggerBot edited projects, added: MW-1.42-notes (1.42.0-wmf.15; 2024-01-23); removed: MW-1.42-notes (1.42.0-wmf.9; 2023-12-12).Jan 22 2024, 5:00 PM
Samwilson moved this task from Backlog to Ready on the Edit Recovery board.Feb 21 2024, 3:53 AM
Nardog mentioned this in T358464: Changes made after discarding changes are not saved.Feb 26 2024, 6:05 AM
GMikesell-WMF subscribed.Feb 26 2024, 10:49 PM

@Samwilson Wikitext is restored to the original state when you click "Discard Changes" as seen in the webm's below. This will be moved to Done. Thanks for all your work!

*Status: ✅PASS
Environment: Beta: 1.42.0-alpha (2b746c7; Local: 1.42.0-alpha (5781b45) 17:58, 26 February 2024
OS: macOS Sonoma 14.2.1
Browser: Firefox 123, Safari 17.3, Edge 122
Skins. Vector 2022
Device: MBA M2
Emulated Device:: n/a
Test Links:**
https://en.m.wikipedia.beta.wmflabs.org/wiki/Dog#
http://localhost:8080/w/index.php?title=Cat&action=edit

✅AC1: https://phabricator.wikimedia.org/T351821

Beta

FirefoxSafariEdge

Local

FirefoxSafariEdge
GMikesell-WMF updated the task description. (Show Details)Feb 26 2024, 10:50 PM
GMikesell-WMF moved this task from QA 🐛 to Done 🏁 on the Community-Tech (CommTech-Kanban) board.
dom_walden mentioned this in T360015: Edit Recovery not working on Safari.Mar 13 2024, 9:47 AM
• JWheeler-WMF closed this task as Resolved.Mar 13 2024, 6:07 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 · Credits