Closed Bug 1760936 Opened 3 years ago Closed 3 years ago

Unloaded tab loads urlbar string in default search engine instead of the correct page

Categories

(Firefox :: Session Restore, defect)

defect

Tracking

()

VERIFIED FIXED
101 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 --- wontfix
firefox101 --- verified

People

(Reporter: tustamido, Assigned: rpl)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Lately, in the last few weeks or months, I've been bothered because some unloaded tabs are loading pages I've never visited. Until I understood what's going on and how to reproduce the issue.

If there was a typed string in urlbar when you discarded a tab, the next time you select that tab will open results in default search engine for the string instead of restoring the page that was loaded when the tab was discarded.

I use Sidebery, but will recommend a lighter extension to reproduce the issue:

  1. Install UnloadTabs.
  2. Open any page, like example.com.
  3. Replace urlbar content by typing anything, like yadayada, but don't press Enter so you don't leave current page.
  4. Right-click the tab and unload it. A different tab will be selected.
  5. Click the unloaded tab to activate it.

Instead of loading the page that was open when you discarded the tab (example.com), it will load search results for yadayada using default search engine (Google in my case).

At least you can still press back button to return to example.com, but it's still a critical bug causing the loading of pages that I never visited, sharing content that may be sensitive (I may have typed anything in urlbar).

Using mozregression, this bug arose when fission was enabled by default:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=383986e2f5cb835a55c47bbd6e15d81035ca0784&tochange=d6e8528f0a936df88369c71f4e390e31a4d621a1

If you disable fission.autostart and restart, restoring a manually discarded tab doesn't work at all, Fx fails to load anything and displays "Hmm. That address doesn’t look right.".

Running mozregression again, this is the range that initially broke restoration of manually unloaded tabs when there was something typed in urlbar:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c734901eec31cab5cff79a272e9e2f266232f58d&tochange=feee34371f46ffd13fa8a7f42ea4a65138bddb7b

I can reproduce this with new profile.

STR

  1. Open two tabs
  2. Select 1st tab and type something in Address bar (should not hit enter)
  3. Select 2nd tab
  4. Execute gBrowser.discardBrowser(gBrowser.tabs[0]) from Browser Console
  5. Select 1st tab

Luca Greco, your patch seems to cause the regression. Could you please look into this?

Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Ever confirmed: true
Flags: needinfo?(lgreco)
Keywords: regression
Regressed by: 1724205

Is this potentially an extensions regressing issue?

Component: Session Restore → Compatibility
Flags: needinfo?(mixedpuppy)
Product: Firefox → WebExtensions

Another str:

  1. Visit webpage (e.g. https://ftp.mozilla.org/pub/firefox/nightly/ )
  2. Type something in Address bar (should not hit enter)
  3. Add a new tab
  4. Visit about:unloads
  5. Click [Unload] button so that the tab of step1 will be unloaded
  6. Select the tab of step1 so that the tab is active

Thanks Alice for the two reduced STR (and apologies for not being able to get to this sooner).

Given the STR from comment 1 and comment 3, this looks definitely like an issue introduced by the fix for Bug 1724205 (and not one introduced by something an extension may be doing).

I'll start investigating it, and I may move it back to "Firefox :: Session Restore" as soon I got some more details from investigating it locally.

(In the meantime I'm clearing Shane's needinfo and keeping mine, to be cleared along with updating this issue, either by attaching a patch or adding some more details about the underlying issue).

Flags: needinfo?(mixedpuppy)
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Attachment #9269917 - Attachment description: Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not a cached history. r?Gijs! → Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not have a cached history. r?Gijs!
Component: Compatibility → Session Restore
Flags: needinfo?(lgreco)
Product: WebExtensions → Firefox

I've looked into it and just attached a patch with a proposed fix.
The idea from the drafted patch is to not store the user typed value set on the browser element into the tab state cache if the user typed value wasn't also already being loading.

Unfortunately things are never that simple, and the tricky part was making sure that the updated version of those checks are still able to pass the existing tests added by some previous fixes in the same area, in particular the test for the change originally introduced to fix Bug 1422588 expects that we would be storing the user typed value if an extension has created a tab and then discarded it right after that, and the test introduced along with that change was getting stuck without also updating the tab cache state when there is no cacheState yet.

I've also looked into adding a new test to cover this issue.

I pushed the attached patch to try here (especially to double-check if there are failures triggered in other existing tests that I should take a look into):

Attachment #9269917 - Attachment description: Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not have a cached history. r?Gijs! → Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not a cached history. r?Gijs!
Attachment #9269917 - Attachment description: Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not a cached history. r?Gijs! → Bug 1760936 - cache userTypedValue for discarded tab when already navigating or the tab does not have a cached history. r?Gijs!
Pushed by luca.greco@alcacoop.it: https://hg.mozilla.org/integration/autoland/rev/d84438a191e4 cache userTypedValue for discarded tab when already navigating or the tab does not have a cached history. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

The patch landed in nightly and beta is affected.
:rpl, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(lgreco)

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #9)

The patch landed in nightly and beta is affected.
:rpl, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

In my opinion it would be reasonable to don't uplift this change and let it ride the train instead.

I also wanted to double-check if there have been any higher intermittent failures rate for pre-existing tests that were covering the previous issues fixed in the same area, I just took a look to the intermittent failures tracked for the test file browser_ext_tabs_discarded.js and Bug 1725701 seems to have increased its intermittent failures rate shortly after this patch have been landed.

That looks suspicious, I'll coordinate with Shane to confirm if that higher intermittent failures rate is related to this change.

Flags: needinfo?(lgreco)
Flags: qe-verify+

I managed to reproduce this on Firefox 100.0(20220428192727) on macOS 11. Verified as fixed on Firefox 101.0b7(20220515185854), Nightly 102.0a1(20220515214519) on Win10 64-bits, Ubuntu 20.04 and macOS 11.

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

Attachment

General

Created:
Updated:
Size: