1745730 - Navigating to the same URL adds a duplicate session history with BFCache in parent

Status: VERIFIED FIXED

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

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:

1. Open about:profiles.
2. Click [Restart normally...] to restart the browser.
3. Open any URL (using https://example.com here) in a new tab.
4. Make sure that the back button is deactivated.
5. Drag the lock icon into the current page.
6. Press Enter key in the address bar.

Actual result:
Step 5 will activate the back button and add an entry to the back menu.
Step 6 will add another entry to the back menu.

Expected result:
Both step 5 and 6 should not activate the back button.

Note that step 2 is required. This is not reproducible with the first run in a fresh profile.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5e13c2a606e299c19f97867e110a9907ceb5c31f&tochange=89aa5231ddb828f9577650abd14b22b299d0f9ab
(edit) This is wrong. See comment #2 for the correct range.

Comment 1

[Masatoshi Kimura [:emk]]

This is even reproducible with in-page links:
3. Open https://www.mozilla.org in a new tab.
4. Make sure that the back button is deactivated.
5. Click the left-top "moz://a" logo.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I have no idea, and with the STR in comment 1, I tried mozregression. The regression range is:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=57367b5d6fcd95b664bf3b926bc510e4b16e83c8&tochange=7d32671931b7c2f5cb24b0f4d62bb6884e484175

And suggested regression point is: https://phabricator.services.mozilla.com/D117185

It makes sense to change this kind of behavior by enabling BFCache.

Comment 3

[Masatoshi Kimura [:emk]]

Yeah, my regression range was wrong somehow. Sorry for the confusion.

Comment 4

[Masatoshi Kimura [:emk]]

I verified that fission.autostart is true on my profile and flipping fission.bfcacheInParent works around the issue on Firefox 95.

Comment 5

[Masatoshi Kimura [:emk]]

Note that step 2 is required. This is not reproducible with the first run in a fresh profile.

This is because Fission staged rollout has no effect until restart (fission.autostart is not a "live" pref).

Comment 6

[Masatoshi Kimura [:emk]]

This bug reproduced since https://hg.mozilla.org/mozilla-central/rev/d683ade21deb with fission.autostart, fission.sessionHistoryInParent, and fission.bfcacheInParent enabled. BFCache in parent has been broken from day one.

Comment 7

[Masatoshi Kimura [:emk]]

This bug is visible from web pages via history.length.

Comment 8

[Masatoshi Kimura [:emk]]

Attachment: Bug 1745730 - Test to make sure that same page naviations do not create a duplicate session history entry. r?smaug

Comment 9

[Masatoshi Kimura [:emk]]

Test only. I have no idea how to fix this.

Comment 10

[Chris Peterson [:cpeterson]]

Olli says this is an edge case in SHIP code.

Comment 11

[Dianna Smith [:diannaS]]

I noticed this was set to leave-open... are we planning on landing the test case before the fix?

Comment 12

[Masatoshi Kimura [:emk]]

I don't know. I added leave-open in case of landing the test case first.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d49e4b68071d
Test to make sure that same page naviations do not create a duplicate session history entry. r=smaug

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d49e4b68071d

Comment 15

[Masatoshi Kimura [:emk]]

This bug is not reproducible if BFCache in parent is disabled even if fission is enabled. Moreover, this bug is reproducible if BFCache in parent is enabled even if fission is disabled.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1745730, disable bfcache when loading the current uri again, r=peterv

The idea is to get us to https://searchfox.org/mozilla-central/rev/25d26b0a62cc5bb4aa3bb90a11f3b0b7c52859c4/docshell/base/nsDocShell.cpp#10824,10829 so that
we can do a replace load.
It should be ok to disable bfcache in all the cases, not only in the cases nsDocShell checks for replace load.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90a3cf110438
disable bfcache when loading the current uri again, r=peterv

Comment 18

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/90a3cf110438

Comment 19

[Ciprian Georgiu, Desktop QA]

I've reproduced this bug using an affected Nightly build (97.0a1, 20211213093143), with STR from comment 0.

The issue is verified as fixed on latest Beta 97.0b9, across OS's: Win 10 x64, macOS 11 and Ubuntu 18.4 x64.