tabs.update with {loadReplace: true} on tabs with moz-extension URLs broken in Firefox 74
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox73 | --- | unaffected |
firefox74 | --- | fixed |
firefox75 | --- | fixed |
People
(Reporter: anonymous30901032, Assigned: mattwoodrow)
Details
(Keywords: regression)
Attachments
(4 files)
(deleted),
video/mp4
|
Details | |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0
Steps to reproduce:
- In Firefox 73, install the 'Tab Suspender' extension: https://addons.mozilla.org/en-US/firefox/addon/ff-tab-suspender/
- In this extensions Options page, deselect 'Automatic Suspend' and click 'Save'. This extension doesn't use native tab discarding, but instead updates a tab to a moz-extension URL when suspended, using the loadReplace property. If the page of a suspended tab is clicked on, the tab will be updated back to the original URL using the loadReplace property.
- Create a new tab, load an https URL in it, select 'Tab Suspender' in that tab's context menu, and then select 'Suspend'.
- Click anywhere on the page to restore/reload it.
- Suspend the tab again, go to a new URL in the same tab, press the back button, restore the suspended tab, and then press the forward button.
- Repeat the previous steps using Firefox 74.
Actual results:
On step 4) with Firefox 73, the moz-extension URL is removed from the tab's history (the back button is unavailable). On step 5) in Firefox 73, the forward arrow is available. In Firefox 74, the back arrow isn't removed on step 4) and the forward arrow is lost on step 5).
Expected results:
In Firefox 74, a tabs.update with the loadReplace property set to true on a tab with a moz-extension URL behaves like a tabs.update without the property set. The loadReplace property should be taken into account, like it was before Firefox 74. Note that because of https://bugzilla.mozilla.org/show_bug.cgi?id=1436321 , loadReplace is necessary to remove moz-extension URLs from a tabs history.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Hello,
I have managed to reproduce the issue on the latest Nightly (75.0a1/20200217095003), Beta (74.0b4/20200216164042), under Windows 10 Pro 64-bit and macOS Catalina 10.15.
The back arrow is not removed after the page is restored and the forward arrow is lost while performing the step 5.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Shane, could this bug be triaged and prioritized please? Thanks
Comment 3•5 years ago
|
||
One note in regards to the repro of this issue, particularly the actual results after step 4) is performed - before and after the regression the back button is available, however after the regression occurs when right clicking on the back button to show history the discarded tab is also listed.
This does not occur before the regression and if the user clicks on the back button they are returned to the new tab.
Attached video with actual results after regression.
As for the regression window, these are the results:
Last good build_date: 2020-01-10 01:45:32.146000
changeset: 325da10270e4bbc644df0fd29a6c3f6ef536d262
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=325da10270e4bbc644df0fd29a6c3f6ef536d262&tochange=558a2ae20a30f33cc1b042dab27ef04eccb4ea58
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: bXHdGMkhTYSIrtqzsfuWvA
First bad build_date: 2020-01-10 01:39:31.316000.
changeset: e166c43d60e363fe86615b0580d34f179db510b3
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=325da10270e4bbc644df0fd29a6c3f6ef536d262&tochange=e166c43d60e363fe86615b0580d34f179db510b3
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: Vx2OsGpiQqarrmHMyzSy0w
If no more details are needed please remove the regression tags.
Comment 4•5 years ago
|
||
Hey Matt, this seem to be from one of your patches for bug 1601779 or bug 1603196, could you please take a look at what's going on?
Our code in question just passes LOAD_FLAGS_REPLACE_HISTORY
to loadURI
:
https://searchfox.org/mozilla-central/rev/df94cd5b/browser/components/extensions/parent/ext-tabs.js#833
(separately, we apparently don't have a test for what loadReplace
is supposed to do)
Comment 5•5 years ago
|
||
I just conflicted with Tomislav, but have some additional info that shows it is not specific to the addon mentioned in OP:
As a note, this can be replicated with other tab discarding addons. I tested with two[1][2]. The quick STR:
- Add a tab discarding addon.
- open a new tab, in this tab:
- go to site A via urlbar
- go to site B via urlbar
- click back button
- change to any other tab
- "discard" the tab from #2 using the context menu on that tab.
- go back to the tab from #2 so it reloads
At this point, the "forward" history has been lost (in tab from #2)
[1] https://addons.mozilla.org/en-US/firefox/addon/discard-tab
[2] https://addons.mozilla.org/en-US/firefox/addon/unload-tab-from-context-menu
Comment 6•5 years ago
|
||
Another thing to note, "discard-tab" uses our API browser.tabs.discard which uses functionality in tabbrowser itself. I just created another STR that uses no addon:
- start firefox
- create new tab
- visit site A
- visit site B
- click back
- select some other tab
- restart firefox
- click on tab created in #2
At this point, forward history is lost.
Given I can reproduce with our own discard functionality, I'm moving this to dom:navigation assuming it is a regression from one of the two mentioned bugs.
Assignee | ||
Comment 7•5 years ago
|
||
I can reproduce the bug as originally filed, but I'm not able to reproduce either of the alternate STR from comments 5 and 6.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D63266
Assignee | ||
Comment 10•5 years ago
|
||
Depends on D63267
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c182ab2cc8ad
https://hg.mozilla.org/mozilla-central/rev/ab59aa76251d
https://hg.mozilla.org/mozilla-central/rev/332b68c9da84
Comment 14•5 years ago
|
||
Should we uplift to beta to avoid shipping this regression?
Assignee | ||
Comment 15•5 years ago
|
||
Comment on attachment 9127414 [details]
Bug 1614847 - Expand test for load types to include a cross-origin case that fails with fission. r?nika
Beta/Release Uplift Approval Request
- User impact if declined: Incorrect session history for extension pages in some cases
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- 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:
Assignee | ||
Updated•5 years ago
|
Comment 16•5 years ago
|
||
Comment on attachment 9127414 [details]
Bug 1614847 - Expand test for load types to include a cross-origin case that fails with fission. r?nika
Fix for a 74 regression, uplift approved for 74 beta 9, thanks.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 17•5 years ago
|
||
bugherder uplift |
Comment 18•5 years ago
|
||
Was this already merged to beta 74.0b9 / nightly?
Because I can still reproduce the issue from duplicate bug 1615302
Description
•