Refreshing session restore page clears session data from the page, preventing session restore
Categories
(Firefox :: Session Restore, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | --- | unaffected |
firefox68 | --- | unaffected |
firefox69 | --- | unaffected |
firefox70 | + | verified |
firefox71 | --- | verified |
People
(Reporter: barret, Assigned: bdahl)
References
(Regression)
Details
(Keywords: dataloss, regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details |
STR
- Trigger session restore (e.g., by force killing the browser).
- Refresh the session restore page
AR
The "Restore Session" button becomes un-clickable. Restarting the browser takes you to about:newtab. The "Restore" option in the history menu is also disabled.
ER
I should be able to restore my session.
This occurs on Nightly but not Release. I don't have an installation of Beta or Dev Edition to compare to.
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Based on this not happening on release, this must have regressed at some point...
Comment 2•5 years ago
|
||
Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.
Comment 3•5 years ago
|
||
Hi guys, I was unable to get a mozregression because of the continuous restarts i needed to trigger the restore session but I did test manually a few builds and I think this is as close as we can get :
First known bad -
Using the builds from the archive I managed to narrow it down a bit to these nightly builds:
70.0a1 (2019-07-24) - last known good build
70.0a1 (2019-07-25) - first known bad build
I hope this helps and maybe theres a way to do a mozregression where you can restart and crash a build without triggering the updates or starting from different temp folders.
anyways hope this helps.
Comment 4•5 years ago
|
||
I found simpler steps to reproduce:
- open about:sessionrestore
- open the devtools console
- get the
sessionData
element by ID, and assign it a valid session restore string as a value (you can use the SessionStore API in the browser console to get one, or just crash a browser a few times yourself properly first, and then copy it from the element's value) - refresh
In good builds, after the refresh the value is still present and the default "restore" button becomes enabled.
In bad builds, it is not.
Regression window:
Brendan, can you please take a look?
Comment 5•5 years ago
|
||
YMMV, but in case it helps, it looks to me like we would not have used the document prototype cache for about:sessionrestore prior to the changes, because the channel's original URI would have been an about: URI, and IsChromeURI would return false, but it looks like now we will use the proto cache, because the page has system principal.
Assignee | ||
Comment 6•5 years ago
|
||
I'll be out of the office next week and won't be able to get to this today. Maybe Brian can take a look for me when he's back monday.
Comment 7•5 years ago
|
||
I can confirm the STR from Comment 4 with: $("#sessionData").value = SessionStore.getBrowserState()
in the web console for about:sessionrestore.
It seems like the .value
on that input used to be persisted from the reload and isn't anymore, which gets us into the early return at https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/browser/components/sessionstore/content/aboutSessionRestore.js#67-73.
Comment 8•5 years ago
|
||
A couple more notes:
- if I move from aboutSessionRestore.xhtml -> aboutSessionRestore.html (along with updating redirector and changing the markup to adapt) the button does remain active after refreshing as you'd expect since we move back off of prototype cache parser, etc.
- There's some direct querying for this input node in session restore files like https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/toolkit/components/sessionstore/SessionStoreUtils.cpp#480-483.
Comment 9•5 years ago
|
||
[Tracking Requested - why for this release]:
This was tripped by a subtle behaviour change in gecko and this might therefore affect other things than just sessionstore - but refreshing about:sessionrestore and losing data that way is bad enough to get this tracked, I suspect.
Brendan is this something you can take on?
Assignee | ||
Comment 11•5 years ago
|
||
(In reply to Liz Henry (:lizzard) from comment #10)
Brendan is this something you can take on?
Yeah, looking into it now.
Assignee | ||
Comment 12•5 years ago
|
||
The session restore page keeps its restore list within a text input field
so that the values are persisted even if the page is refreshed. When form
elements were loaded with the prototype cache we didn't call
DoneCreatingElement after creating the element, which means the form values
weren't restored.
The list of elements that require DoneCreatingElement and DoneAddingChildren
to be called was in three (now four) different places, so I moved them to
a central spot in nsIContent to share in all locations. This also highlighted
that the check for <output> nodes is missing from the XML content sink.
Comment 13•5 years ago
|
||
Comment 14•5 years ago
|
||
Backed out changeset 5e9663384022 (Bug 1575620) for causing bustages in builds/worker/workspace/build/src/obj-firefox/dist/include/nsIContent.h CLOSED TREE
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265407311&repo=autoland&lineNumber=13462
Comment 15•5 years ago
|
||
Comment 17•5 years ago
|
||
Comment 18•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 20•5 years ago
|
||
Comment on attachment 9090770 [details]
Bug 1575620 - Fix refreshing session restore when using prototype cache. r=smaug
Beta/Release Uplift Approval Request
- User impact if declined: Potential to lose the option to restore tabs if the user reloads the session restore page.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 1) open about:sessionrestore
- open the devtools console
- run
$("#sessionData").value = SessionStore.getBrowserState()
in devtools - reload the page
- the restore button should still be active
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): This patch changes (fixes) how some elements were being handled in XHTML, which could have unknown side affects. I don't think this is likely though, since it now moves XHTML handling more inline with HTML handling.
- String changes made/needed:
Assignee | ||
Updated•5 years ago
|
Comment on attachment 9090770 [details]
Bug 1575620 - Fix refreshing session restore when using prototype cache. r=smaug
Avoid a new session store regression in 70.
OK for uplift for beta 7.
Comment 22•5 years ago
|
||
bugherder uplift |
Comment 23•5 years ago
|
||
Hello,
Reproduced the issue with Firefox 70.0a1 (20190821215524) on Windows 10x64.
The issue is verified fixed with Firefox 71.0a1 (20190915214245) and Firefox 70.0b7 (20190915235853) on Windows 10x64, macOS 10.14 and Ubuntu 18.04.
Description
•