Closed Bug 1615302 Opened 5 years ago Closed 5 years ago

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).

Categories

(Firefox :: Session Restore, defect, P2)

74 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 76
Tracking Status
firefox-esr68 --- unaffected
firefox73 --- unaffected
firefox74 --- wontfix
firefox75 --- verified
firefox76 --- verified

People

(Reporter: anonymous30901032, Assigned: mattwoodrow)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

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.

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

Status: UNCONFIRMED → NEW
Component: Untriaged → Request Handling
Ever confirmed: true
Product: Firefox → WebExtensions

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

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(virginia.balducci)
Resolution: --- → DUPLICATE

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

Status: RESOLVED → REOPENED
Flags: needinfo?(tomica)
Resolution: DUPLICATE → ---

Thanks for the ping.

Flags: needinfo?(tomica)

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

Has Regression Range: --- → yes
Has STR: --- → yes
Regressed by: 1601779

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

Flags: needinfo?(mixedpuppy)

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.

Flags: needinfo?(virginia.balducci)
Flags: needinfo?(mixedpuppy)
Priority: -- → P2
Flags: needinfo?(matt.woodrow)

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?

Flags: needinfo?(matt.woodrow) → needinfo?(alchen)

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://..

Assignee: nobody → matt.woodrow

(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?

Flags: needinfo?(alchen) → needinfo?(mdeboer)

(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?

Flags: needinfo?(mdeboer) → needinfo?(matt.woodrow)

(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).

Flags: needinfo?(matt.woodrow)

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

Flags: needinfo?(matt.woodrow)
Flags: qe-verify+
Component: Request Handling → Session Restore
Product: WebExtensions → Firefox
Pushed by mwoodrow@mozilla.com: 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
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 76

Matt, can we uplift this to 75?

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:
Flags: needinfo?(matt.woodrow)
Attachment #9133072 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

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

Attachment #9133072 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

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.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: