arni2033 Reporter
	Description • 8 years ago
str

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
There're 3 scenarios: A, B and C

>>>
STR_1:
1. Set about:blank as homepage
2. Right-click Australis menu, click "Customize..."
3.A) Press Alt+Home
3.B) Press Ctrl+K
3.C) Open new tab, type text "ya.ru" in urlbar, select it, drop it onto customize tab in tabs toolbar

AR:  (A) - the tab loses its favicon.   (B),(C) - infinite gray spinner is displayed in <tab>
ER:  Either X or Y
 X) If navigation supposed to work,
   (A) - about:blank should open,   (B) - about:home should open,   (C) - https://ya.ru should open
 Y) If navigation isn't supposed to work, such actions should not affect favicon in <tab>
Either way, behavior is wrong.

(A) is regression from bug 1249608. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=62cc4b503870cf7be4ae0c530a3d7ab99c8585b9&tochange=ca83a163091aff314159011e639520f25daa3a99

(B),(C) are regression from bug 1249608. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=ca83a163091aff314159011e639520f25daa3a99&tochange=eff6731606da854ee031a06a64191484c09e72e4

	ovidiu boca[:Ovidiu]
	Comment 1 • 8 years ago

Based on the fact that are 2 regressions ranges I think this bug should be splitted in 2 separate issues.    

Dão Gottwald can you please take a look at this, the regression range shows that your fix cause this. 
 https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=62cc4b503870cf7be4ae0c530a3d7ab99c8585b9&tochange=ca83a163091aff314159011e639520f25daa3a99

Sebastian Hengst can you please take a look at this, the regression range shows that your fix cause this. 
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=ca83a163091aff314159011e639520f25daa3a99&tochange=eff6731606da854ee031a06a64191484c09e72e4

	ovidiu boca[:Ovidiu]
	Comment 2 • 8 years ago

I saw that for Dão is this bug:  https://bugzilla.mozilla.org/show_bug.cgi?id=1327178. I will clear the NI? flag.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 3 • 8 years ago

The first commit ca83a163091aff314159011e639520f25daa3a99 busted the scripts which followed the code with the missing parenthesis. The followup eff6731606da854ee031a06a64191484c09e72e4 only fixed that and what started failing on that commit should have been totally broken in the previous one.

	Philip Chee
	Comment 4 • 8 years ago

Firefox Australis "customize page" is separate from toolkit "customize page". Moving back to Firefox

	Justin Dolske [:Dolske]
	Comment 5 • 8 years ago

(In reply to arni2033 [Please stop 'improving' Firefox] from comment #0)

> 3.A) Press Alt+Home
> 3.B) Press Ctrl+K
> 3.C) Open new tab, type text "ya.ru" in urlbar, select it, drop it onto
> customize tab in tabs toolbar
> 
> AR:  (A) - the tab loses its favicon.   (B),(C) - infinite gray spinner is
> displayed in <tab>

(B) works as I'd expect -- nothing happens (no change to page, favicon, or spinner). It normally focuses the search field, and since that's not really present in customization mode there's nothing it should do. (IIRC we made it open about:home when you've removed the search box, so that the keyboard shortcut does something useful, but I don't think that special case needs to work here.)

(A) and (C) both seem to cause problems, and feels like something we should fix.


> (A) is regression from bug 1249608. Regression range:
> > https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=62cc4b503870cf7be4ae0c530a3d7ab99c8585b9&tochange=ca83a163091aff314159011e639520f25daa3a99
> 
> (B),(C) are regression from bug 1249608. Regression range:
> > https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=ca83a163091aff314159011e639520f25daa3a99&tochange=eff6731606da854ee031a06a64191484c09e72e4

I strongly suspect bug 1014185 is the common regressing bug. The code changes in bug 1249608 seem unrelated to this.

Dao, can you take a look?

	Dão Gottwald [:dao]
	Comment 6 • 8 years ago

The problem appears to be that we're forcing the browser to be in-process for performance reasons (bug 1014185 comment 9). I thought we'd automatically update the browser's remoteness when loading something else but apparently this expectation doesn't quite hold. Maybe mconly knows the right way to fix this?

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 8 • 8 years ago

This looks like the simplest fix here. Feel okay reviewing this, dao?

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 9 • 8 years ago

(See commit message for explanation of the problem / solution).

	Dão Gottwald [:dao]
	Comment 10 • 8 years ago
mozreview-review

Comment on attachment 8825482 [details]
Bug 1327210 - Make sure we can flip remoteness on a customizemode tab if the user somehow finds a way of navigating away.

https://reviewboard.mozilla.org/r/103610/#review104510

Thanks! I hadn't realized we utilized SessionStore to flip remoteness.

	Pulsebot
	Comment 11 • 8 years ago

Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7a5b5037888
Make sure we can flip remoteness on a customizemode tab if the user somehow finds a way of navigating away. r=dao

	Carsten Book [:Tomcat]
	Comment 12 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/d7a5b5037888

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 7 years ago

Probably too late for 51 at this point (we're spinning the RC build today), but it sounds like we should get this uplifted to Aurora for 52 at least.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 14 • 7 years ago

Yeah, I think this is pretty safe. I'll request uplift.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 15 • 7 years ago

Comment on attachment 8825482 [details]
Bug 1327210 - Make sure we can flip remoteness on a customizemode tab if the user somehow finds a way of navigating away.

Approval Request Comment
[Feature/Bug causing the regression]:

Bug 1014185 

[User impact if declined]:

Users with e10s enabled who enter customize mode and find a way of causing the browser to browse away (perhaps with the Alt-Home shortcut, for example), will find that the browser tab doesn't do anything, and just shows the tab throbber.

[Is this code covered by automated tests?]:

Customize mode certainly is, as well as the SessionStore stuff that this patch modifies. The problem that this bug was filed for, however, is not covered by an automated test.

[Has the fix been verified in Nightly?]:

Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 

STR are in https://bugzilla.mozilla.org/show_bug.cgi?id=1327210#c0

[List of other uplifts needed for the feature/fix]:

None.

[Is the change risky?]:

No.

[Why is the change risky/not risky?]:

It's a small change that affects only customize mode (which is entered pretty rarely) in a very rare circumstance (navigating away from customize mode is pretty hard to do).

[String changes made/needed]:

None.

	Gerry Chang [:gchang]
	Comment 16 • 7 years ago

Too late for 51. Mark 51 won't fix.

	Julien Cristau [:jcristau]
	Comment 17 • 7 years ago

Comment on attachment 8825482 [details]
Bug 1327210 - Make sure we can flip remoteness on a customizemode tab if the user somehow finds a way of navigating away.

fix regression when moving away from customize mode with e10s, aurora52+

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/f273dcc18ec6

	Sajedul Islam
	Comment 19 • 7 years ago

I have reproduced this bug with Nightly 49.0a1 (2016-05-26) on Windows 8.1 , 64 Bit ! 

This bug's fix is Verified with latest Beta And Aurora !

Build   ID  20170220070057
User Agent  Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build   ID  20170222004022
User Agent  Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

	Jared Wein [:jaws] (please needinfo? me)
	Comment 20 • 7 years ago

Thanks!

	Sajedul Islam
	Comment 21 • 7 years ago

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #20)
> Thanks!

You Are Welcome :)