1615302 - Issues with the back/forward buttons on new tabs created with moz-extension URLs that are updated to different URLs through the location bar (Firefox 74).

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P2)

Description

[anon]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

1. Install the 'Group Speed Dial' extension which creates a new tab page that is a moz-extension URL: https://addons.mozilla.org/en-US/firefox/addon/groupspeeddial/
2. Create a new tab (Ctrl+T), which should be the 'Group Speed Dial' new tab page.
3. Type "https://www.duckduckgo.com" in the location bar and hit enter.
4. Click on the back arrow after the new page has loaded.
5. Type "https://www.startpage.com" in the location bar and hit enter.
6. Press the back arrow twice after the new page has loaded or right click the back arrow and select 'Group Speed Dial'.

Actual results:

On step 4) with Firefox 74, the back button will not allow access to the previous page. On step 6) the forward history of the tab is lost.

Expected results:

Firefox 73 did not have these issues. This new behavior only seems to happen when a new tab is created with a moz-extension URL, not if the moz-extension URL is loaded into the tab after it is created. Also, the URL has to be changed through the location bar after the tab is created with the moz-extension URL. Note that the moz-extension URL could be replaced with a file URL and the behavior would also be the same on steps 4) and 6) with Firefox 74.

Comment 1

[juraj.masiar]

Thank you for reporting it! I'm author of GroupSpeedDial and I was just about to report it :), really annoying bug!
The bug can be actually reproduced from any addon page - changing the URL to any http(s) page will then "stuck" the back button.

Comment 2

[Virginia (please send need info request to Brindusa Tot)]

Hi,

Thanks for reporting this bug to us.

I was able to reproduce it on Firefox Nightly 75.0a1 (2020-02-18) and Firefox Beta 74.0b1 (64-bit) on Windows 8.
It was not reproducible on Firefox Release 73.0 and 73.0.1

I will mark this as NEW and add it to the "WebExtensions- Request Handling" component so their team can investigate and provide their insights.

Request Handling team: please route this ticket to the corresponding team if needed.

Regards,
Virginia

Comment 3

[Tomislav Jovanovic :zombie]

Hi Virginia, this seems similar to bug 1614847, can you please confirm the regression window is the same?

Comment 4

[anon]

Though https://bugzilla.mozilla.org/show_bug.cgi?id=1614847 was just fixed in Firefox 75, this bug is still active in Firefox 75.

Comment 5

[Julien Cristau [:jcristau]]

Reopening per comment 4 and https://bugzilla.mozilla.org/show_bug.cgi?id=1614847#c18

Comment 6

[Tomislav Jovanovic :zombie]

Thanks for the ping.

Comment 7

[Alice0775 White]

I can reproduce on Nighyky75.0a1(20200305095541) Windows10.

STR

1. Open uBlock Origin dashboard
2. Open web page(e.g. http://ftp.mozilla.org/pub/firefox/nightly/) from location bar
3. Click Back button or Alt+←

Actual results:
Navigation back does not work.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=325da10270e4bbc644df0fd29a6c3f6ef536d262&tochange=a9b06feb5942c5e16bcf9adefe84e081edc402c7

Regressed by: Bug 1601779, Bug 1603196

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Shane, could you prioritize this bug? It seems that it was not a duplicate of bug 1614847 or that the fix uplifted there was not covering all cases? Given that we ship 74 in 4 days, it doesn't seem fixable for our release date but maybe a safe fix could be evaluated in a dot release. Thanks

Comment 9

[Tomislav Jovanovic :zombie]

Thanks Alice!

Hey Matt, can you please take a look at this? Seems to be regressed by by the same patches as bug 1614847, but the fix there didn't address it.

Comment 10

[Matt Woodrow (:mattwoodrow)]

It looks like SessionHistory is correct, but when you try to go back to the moz-extension URL it calls LoadInOtherProcess with TabState that includes a userTypedValue for the current tab.

The content process restore code thinks that we should be loading the moz-extension URL (and calls nsDocShell::SetCurrentURI with that), but then we override with the userTypedValue of whatever the current tab is.

I suspect this is something to do with calling navigateAndRestore during the middle of loading (when we load the http url from the moz-extension one).

It looks like we clear userTypedValue from the browser when we finish loading the http URL, but nothing clears it from the cached TabState.

Alphan, do you have any ideas as to how this is supposed to work?

Comment 11

[Matt Woodrow (:mattwoodrow)]

I think the issue is that when we do a process-switch mid-load browser.didStartLoadSinceLastUserTyping is true (since we've started the load), whereas it's false when we do a pre-emptive process switch.

That causes us to set tabData.userTypedClear=1, and it doesn't appear that we clear it anywhere.

When we go back (from the http url to the moz-extension one), this is still set in the tabData and we override using the userTypedValue=http://..

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1615302 - Don't use userTypedValue as the URL to restore when doing a remoteness update through SessionStore. r?alchen

Comment 13

[Alphan Chen [:alchen]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #11)

I think the issue is that when we do a process-switch mid-load browser.didStartLoadSinceLastUserTyping is true (since we've started the load), whereas it's false when we do a pre-emptive process switch.

That causes us to set tabData.userTypedClear=1, and it doesn't appear that we clear it anywhere.

When we go back (from the http url to the moz-extension one), this is still set in the tabData and we override using the userTypedValue=http://..

I agreed with you. I also saw the same symptom.
In this case, tabData.userTypedClear is true and we also have tabData.userTyped.
In restoreTabContent, sessionRestore will try to do a "loadURI(tabData.userTyped)" rathen than "history.reloadCurrentEntry".

Mike, could you also input your comment?

Comment 14

[Mike Conley (:mconley) (:⚙️)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #11)

I think the issue is that when we do a process-switch mid-load browser.didStartLoadSinceLastUserTyping is true (since we've started the load), whereas it's false when we do a pre-emptive process switch.

Is there a reason why we don't call browser.urlbarChangeTracker.startedLoad(); in the pre-emptive process switch case?

Comment 15

[Matt Woodrow (:mattwoodrow)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #14)

(In reply to Matt Woodrow (:mattwoodrow) from comment #11)

I think the issue is that when we do a process-switch mid-load browser.didStartLoadSinceLastUserTyping is true (since we've started the load), whereas it's false when we do a pre-emptive process switch.

Is there a reason why we don't call browser.urlbarChangeTracker.startedLoad(); in the pre-emptive process switch case?

I don't know of one, maybe we should? We currently only call it in response to the nsIWebProgressListener events, and I'm not sure what else might be affected by starting it early.

Note that this bug regressed as a result of using the pre-emptive switch code less often (and I'd like to get rid of entirely in the future).

Comment 16

[Shane Caraveo (:mixedpuppy)]

Matt, is this ready to land, and should this uplift to 75?

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3166149f37f1
Don't use userTypedValue as the URL to restore when doing a remoteness update through SessionStore. r=alchen,mconley

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3166149f37f1

Comment 21

[Julien Cristau [:jcristau]]

Matt, can we uplift this to 75?

Comment 22

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9133072 [details]
Bug 1615302 - Don't use userTypedValue as the URL to restore when doing a remoteness update through SessionStore. r?alchen

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect back/forward button state in some cases with extension pages.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 23

[Julien Cristau [:jcristau]]

Comment on attachment 9133072 [details]
Bug 1615302 - Don't use userTypedValue as the URL to restore when doing a remoteness update through SessionStore. r?alchen

approved for 75.0b10

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/aca558d69c1b

Comment 25

[Marcela (Please NI request to Brindusa Tot)]

Reproduced the initial issue on Release version 74.0 Build ID 20200309095159
Verified - Fixed in latest Nightly 76.0a1 Build ID 20200326093308 using Windows 10, macOS 10.14 and Ubuntu 18.04

Comment 26

[Marcela (Please NI request to Brindusa Tot)]

Verified - Fixed in Beta version 75.0b10 Build ID 20200326191140 using Windows 10, macOS 10.14 and Ubuntu 18.04. Changed flags and status accordingly.