Closed Bug 1609475 Opened 3 months ago Closed 1 month ago

Regression when navigating multiple subframes through history simultaneously

Categories

(Core :: DOM: Navigation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla75
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:

  1. Load a page with two iframes, both of which can be navigated using a link, such as https://five-cartwheel.glitch.me/.
  2. Click on the link in iframe 1, followed by the link in iframe 2
  3. 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.

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.

Flags: needinfo?(peterv)
Summary: Multiple Subframe History Navigation → Regression when navigating multiple subframes through history silmultaneously

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 and aLoadResult, we should probably be using a nsTArray<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.

Priority: -- → P1
Summary: Regression when navigating multiple subframes through history silmultaneously → Regression when navigating multiple subframes through history simultaneously

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: nobody → bugs

Tested locally with and without Fission and with and without the shistory-in-parent pref.

Attachment #9126442 - Attachment is obsolete: true
Attachment #9127551 - Attachment is obsolete: true
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2bd2a22267d6
history.go(>1) should possibly load several iframes, r=annyG

Oh, hmm, is it a silly mistake.

Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c6a6c8deaf0
history.go(>1) should possibly load several iframes, r=annyG
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75
Flags: needinfo?(peterv)
Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bd301d8127f
Expand on the comment about special behaviour in docshell/test/navigation/file_bug1609475.html, r=smaug
QA Whiteboard: [qa-75b-p2]
You need to log in before you can comment on or make changes to this bug.