Regression when navigating multiple subframes through history simultaneously
Categories
(Core :: DOM: Navigation, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox73 | --- | wontfix |
firefox74 | --- | wontfix |
firefox75 | --- | fixed |
People
(Reporter: nika, Assigned: smaug)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
Repro Webpage: https://five-cartwheel.glitch.me/
STR:
- Load a page with two iframes, both of which can be navigated using a link, such as https://five-cartwheel.glitch.me/.
- Click on the link in iframe 1, followed by the link in iframe 2
- Click and hold the "back" button, and select the history entry 2 entries ago.
Expected:
Both subframes navigate back, as-if the "back" button was pressed twice.
Actual:
Only the second frame navigates back, as-if the "back" button was pressed once.
Additional Notes:
- The currently active session history entry is the selected entry, however the state of the first iframe is out of sync.
- Pressing the "forward" button causes the first iframe to re-load
This was regressed at some point since Firefox 71 (the "stable" version I happen to have handy). I'm guessing it was regressed when we merged the session history + fission changes from ash into mozilla-central.
Reporter | ||
Comment 1•5 years ago
|
||
I believe this is caused due to the LoadEntryResult
outparameter from nsSHistory::LoadDifferingEntries
and friends being a single entry to load, rather than a list of loads to perform. https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/docshell/shistory/nsSHistory.cpp#1475-1477
Rather than using a separate aDifferenceFound
and aLoadResult
, we should probably be using a nsTArray<LoadEntryResult>
outparameter, and appending to it when differences are found.
Reporter | ||
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Ugh. Our session history testcases are so incomplete :-(.
(In reply to :Nika Layzell (ni? for response) from comment #1)
Rather than using a separate
aDifferenceFound
andaLoadResult
, we should probably be using ansTArray<LoadEntryResult>
outparameter, and appending to it when differences are found.
I think that would be the right fix, which is going to be more complicated with fission enabled of course.
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
peterv, would you have time to look at this?
I think we should get this fixed before doing more the Fission changes, just make it easier to test everything.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Tested locally with and without Fission and with and without the shistory-in-parent pref.
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
Assignee | ||
Comment 7•5 years ago
|
||
Updated•5 years ago
|
Comment 9•5 years ago
|
||
Backed out for failing test_sessionhistory.html
Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=2bd2a22267d6585b9e2a420cd48ec23b7c91c475&selectedJob=289977227
Tier1 failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=289975211&repo=autoland&lineNumber=2799
Backout: https://hg.mozilla.org/integration/autoland/rev/e09bb8097c62c6e0f966a42e0a2c02a9ccadd1a8
Assignee | ||
Comment 10•5 years ago
|
||
Oh, hmm, is it a silly mistake.
Assignee | ||
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Updated•5 years ago
|
Description
•