1277060 - When typing in the URL bar if a parent process page is loaded and loading about:home using the toolbar button, shortcut, a bookmark, anything besides the url bar - the location bar is not cleared

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[:Gijs (he/him)]

STR (originally from bug 1276117 comment 19 by arni)

1. open a new tab
2. type in the location bar
3. click home button (or hit alt-home)

ER:
location bar is cleared

AR:
location bar is not cleared

This does not happen if using a private window to reproduce these steps.

Alternative steps (that probably also reproduce in a private window):

1. open a new tab
2. type in "about:preferences" and hit enter
3. type in the location bar some more, replacing about:preferences
4. click home button (or hit alt-home)

ER:
location bar is cleared

AR:
location bar is not cleared



This has to do with how we don't clear the location bar for remoteness-switch-based loads of initial pages. There isn't an easy way, that I can see, to distinguish these from exactly the case we fixed in bug 1241085, where we're trying to avoid clearing user-typed info when about:privatebrowsing (or about:home) first loads in a new (private) window. Mike/Mike, thoughts/ideas/suggestions?

If we make about:newtab remote, or about:home loadable in the parent process, this would go away, I think. about:privatebrowsing already loads remotely, which is why the first set of STR don't reproduce in private browsing windows.

Comment 1

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #0)
> If we make about:newtab remote, or about:home loadable in the parent
> process, this would go away, I think. about:privatebrowsing already loads
> remotely, which is why the first set of STR don't reproduce in private
> browsing windows.

Making about:newtab remote sounds like the path-of-least-wasted-effort to me. I've seen bug 1272342 before and Mike looks involved ;-)

Comment 2

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

(In reply to Mike de Boer [:mikedeboer] from comment #1)
> (In reply to :Gijs Kruitbosch from comment #0)
> > If we make about:newtab remote, or about:home loadable in the parent
> > process, this would go away, I think. about:privatebrowsing already loads
> > remotely, which is why the first set of STR don't reproduce in private
> > browsing windows.
> 
> Making about:newtab remote sounds like the path-of-least-wasted-effort to
> me. I've seen bug 1272342 before and Mike looks involved ;-)

I agree that this would help alleviate this somewhat, but it wouldn't fix the underlying bug, still being exposed by about:preferences.

(In reply to :Gijs Kruitbosch (no reviews until June 6) from comment #0)
> 
> This has to do with how we don't clear the location bar for
> remoteness-switch-based loads of initial pages.

Perhaps we're being too strict here. Like, we really do only care about _initial_ page loads here (even if the browser has switched remoteness), right?

In our example case with about:preferences, since we know that about:preferences has loaded once before in this tab, can't we use that information and change our behaviour accordingly?

Or are we walking backwards then, and going back to our more complicated world that we gleefully escaped from?

Comment 3

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #2)
> (In reply to :Gijs Kruitbosch (no reviews until June 6) from comment #0)
> > 
> > This has to do with how we don't clear the location bar for
> > remoteness-switch-based loads of initial pages.
> 
> Perhaps we're being too strict here.

You mean too lenient / not strict enough? :-)

> Like, we really do only care about
> _initial_ page loads here (even if the browser has switched remoteness),
> right?

Yes, assuming we treat about:newtab as the initial load and not what people load in the tab afterwards... and that then also doesn't jive well with bug 610357... but hey, separate bug...

> In our example case with about:preferences, since we know that
> about:preferences has loaded once before in this tab, can't we use that
> information and change our behaviour accordingly?

How do we know this? Does sessionstore actually guarantee that we restore history state to the new browser before we start loading the new URI in it? Or do you think we should have some kind of expando on the browser that tracks the number of loads we've had or something? Not sure where you're going with this one. :-)

> Or are we walking backwards then, and going back to our more complicated
> world that we gleefully escaped from?

Not necessarily. I think the problem we used to have was twofold:
1) we had a complex implementation using the seemingly arbitrary incrementing/decrementing of this otherwise opaque-looking integer
2) we broke stuff in subtle ways by the whole browser remoteness switching malarkey.

We fixed (1), and we're halfway through addressing (2), but we're still running into it here. I don't know that it's easy to address that.

Comment 4

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

(In reply to :Gijs Kruitbosch from comment #3)
> How do we know this? Does sessionstore actually guarantee that we restore
> history state to the new browser before we start loading the new URI in it?

I _think_ so - SessionStore.navigateAndRestore is used to flip remoteness, and do the "brain transplant" of history down to the newly created browser. Once the history is sent down, we then do the load.

So I _think_ we have some kind of guarantee here.

Comment 5

[:Gijs (he/him)]

Found this again because of bug 1453723 (not actually related, but originates from bug 1276117 as well) - Mike, like a month ago you asked me about specific concerns I had with new-tab/about:home loading in a different process to regular webpages. This is the type of thing that'd become more common / we'd have issues with. That is:

(In reply to :Gijs (he/him) from comment #0)
> If we make about:newtab remote, or about:home loadable in the parent
> process, this would go away, I think.

This has happened now, so I would expect the first set of STR don't reproduce anymore - but they do for me, in 62 beta...

The second set of STR from comment #0 definitely still reproduces, because about:preferences is in the parent process and the homepage isn't. However, after putting about:home/about:newtab in their own process, I'd expect issues here to be worse because they will happen if you hit the home button from *any* webpage.

Comment 6

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

Yeah, with the separate content process for Activity Stream enabled, I'm certainly seeing this bug.

STR:

1. Ensure browser.tabs.remote.separatePrivilegedContentProcess is set to true (if not, set to true and restart)
2. Ensure clicking the Home button takes you to Activity Stream
3. Browse any tab to https://www.mozilla.org
4. Type anything into the URL bar
5. Hit the Home button

ER:

The URL bar should be cleared.

AR:

The URL bar is not cleared.

Comment 7

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

Do you have the cycles to look at this, Jay?

Comment 8

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

Attachment: Bug 1277060 - Tell the URLbar change tracker that we've started a load when flipping remoteness. r?Gijs

Normally, the URLbar change tracker hears that loads start via the onStateChange
method of the tab's TabProgressListener. However, during a remoteness flip, that
state change is never fired (since the content process goes away). This patch makes
us note that we've started a load for the remoteness-flipping special-case.

Comment 9

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

Attachment: Bug 1277060 - Rename initialPageLoadedFromURLBar to initialPageLoadedFromUserAction. r?Gijs

There are cases where we might want to set this from a non-URL bar user action -
for example, when clicking on the Home button.

Comment 10

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

Attachment: Bug 1277060 - Ensure the URL bar gets cleared if Home button sends us to an initial page. r?Gijs

Depends on D17407

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17a249eff0c7
Rename initialPageLoadedFromURLBar to initialPageLoadedFromUserAction. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/89259bb285eb
Ensure the URL bar gets cleared if Home button sends us to an initial page. r=Gijs

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/17a249eff0c7
https://hg.mozilla.org/mozilla-central/rev/89259bb285eb

Comment 13

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 49.0a1 (2016-05-31) on Windows 10, 64 Bit and the fix of the bug is verified with latest Nightly 67.0a1!

Build ID : 20190202213603
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0