Mike de Boer [:mikedeboer] Assignee
	Description • 9 years ago

I noticed that the Contacts tab doesn't show and hide anymore after logging in and out with FxA. Only after a browser restart does the tab appear.

This is critical and greatly reduces the usability of Loop/ Hello, because there's no way to access the direct calling feature this way.

	Mike de Boer [:mikedeboer] Assignee
	Comment 1 • 9 years ago

I introduced this bug in my patch for bug : here `shouldComponentUpdate` was introduced for the TabView component in panel.jsx. It checks for changed properties, but only for the components' changed state, not its properties. The `buttonsHidden` property is the one we need to account for, because it determines whether the Contacts tab buttons is shown.

This unfortunately affects release, Fx 36, and IMHO warrants a point release.

	Mike de Boer [:mikedeboer] Assignee
	Comment 3 • 9 years ago

The thing I don't understand yet is why `browser/components/loop/test/mochitest/browser_toolbar.js` didn't catch this bug... I'm investigating this atm.

	Mike de Boer [:mikedeboer] Assignee
	Comment 4 • 9 years ago

[Tracking Requested - why for this release]: This is impacting the FTU experience for Loop/ Hello direct calling. At the moment the contacts tab does _not_ show immediately after login with an Fx Account, only after a full browser restart.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 5 • 9 years ago

Can we write a browser chrome test that would have caught this?

	Mike de Boer [:mikedeboer] Assignee
	Comment 6 • 9 years ago

[Tracking Requested - why for this release]: see comment 4.

	Mike de Boer [:mikedeboer] Assignee
	Comment 8 • 9 years ago

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #5)
> Can we write a browser chrome test that would have caught this?

Yes, I attached a test update patch just now. I was hinting at this in comment 3.

	Mark Banner (:standard8)
	Comment 9 • 9 years ago

Comment on attachment 8569793 [details] [diff] [review]
Patch v1: fix for making the Loop contacts tab show and/ or hide when the user logs in or out of FxA

Review of attachment 8569793 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, I've given this a few runs through and it looks good.

	Mike de Boer [:mikedeboer] Assignee
	Comment 10 • 9 years ago

Thanks Mark! Pushed to fx-team:

https://hg.mozilla.org/integration/fx-team/rev/52e496c8a9fa
https://hg.mozilla.org/integration/fx-team/rev/cb5051165c85

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/52e496c8a9fa
https://hg.mozilla.org/mozilla-central/rev/cb5051165c85

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 12 • 9 years ago

 I know we are tracking this for release and there is probably going to be a 36.0.1.  Can you nominate it for uplift? Thanks!

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 13 • 9 years ago

ni Sylvestre to review this bug for inclusion in 36.0.1, which gtb early this week.

Mike - It's preferable to get an uplift request by early Monday so that we can include the fix in 36.0.1 and 37 Beta 2.

	Sylvestre Ledru [:Sylvestre]
	Comment 14 • 9 years ago

It is on my track list and we exchanged with Liz on this topic, no worries!

	Mike de Boer [:mikedeboer] Assignee
	Comment 15 • 9 years ago

Comment on attachment 8569793 [details] [diff] [review]
Patch v1: fix for making the Loop contacts tab show and/ or hide when the user logs in or out of FxA

Approval Request Comment
[Feature/regressing bug #]: bug 1121210
[User impact if declined]: After login with FxA, the user won't see the contacts tab inside the Loop panel _until_ he or she restarts the browser. This should happen instantly after login.
[Describe test coverage new/current, TreeHerder]: landed on m-c, adjusted unit tests to fail when this patch is not applied.
[Risks and why]: minor
[String/UUID change made/needed]: n/a

	Mike de Boer [:mikedeboer] Assignee
	Comment 16 • 9 years ago

Comment on attachment 8569960 [details] [diff] [review]
Patch 2: extend Loop toolbarbutton tests to check for correct panel states upon opening

See comment 15 for approval request. This patch contains the updated unit tests mentioned therein.

	Mike de Boer [:mikedeboer] Assignee
	Comment 17 • 9 years ago

Sylvestre, you missed one ;)

	Sylvestre Ledru [:Sylvestre]
	Comment 18 • 9 years ago

Comment on attachment 8569793 [details] [diff] [review]
Patch v1: fix for making the Loop contacts tab show and/ or hide when the user logs in or out of FxA

Oups, sorry

	Sylvestre Ledru [:Sylvestre]
	Comment 19 • 9 years ago

checkin-needed is not needed for branch uplifts.

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/c1b3c8c553e9
https://hg.mozilla.org/releases/mozilla-aurora/rev/4b51b9e2c088

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 9 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/d5def3938b6e
https://hg.mozilla.org/releases/mozilla-beta/rev/824656d7ad0d

	Ryan VanderMeulen [:RyanVM]
	Comment 22 • 9 years ago

https://hg.mozilla.org/releases/mozilla-release/rev/7a4c981abfb6
https://hg.mozilla.org/releases/mozilla-release/rev/608bab8e4868

	Vasilica Tamas, QA [:vtamas]
	Comment 23 • 9 years ago

I was able to reproduce this issue on Firefox 39.0a1 (2015-02-26) using Windows 7 x64.
Verified fixed on Firefox 39.0a1 (2015-03-03), Firefox Dev Edition 38.0a2 (2015-03-03) and Firefox 37 Beta 2 (20150302192546) using Windows 7 x64, Ubuntu 13.10 x64 and Mac OS X 10.8.5.

I have noticed a potential issue: using a clean profile, after the first login with FxA, the Contacts tab appears with a delay of about 3 seconds.
 
Should I file another bug for this issues?

	Mike de Boer [:mikedeboer] Assignee
	Comment 24 • 9 years ago

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #23)
> I have noticed a potential issue: using a clean profile, after the first
> login with FxA, the Contacts tab appears with a delay of about 3 seconds.
>  
> Should I file another bug for this issues?

Hi Vasilica, thanks for verifying this issue! It's expected that the contacts tab appears with a small delay. The FxA login process isn't the fastest, so no need to file an issue for this.

	Dan Mosedale (:dmosedale, :dmose)
	Comment 25 • 9 years ago

Mike, would naïve users expect that delay? If not, maybe a new bug suggesting a spinner implementation would make sense… what do you think?

	Mike de Boer [:mikedeboer] Assignee
	Comment 26 • 9 years ago

Hmm, you've got a point but that spinner would need to be on the toolbar button methinks.

Bugs are cheap: bug 1139028

	Vasilica Tamas, QA [:vtamas]
	Comment 28 • 9 years ago

Verified fixed on Firefox 36.0.1 build 2 (20150305021524) using Windows 7 x64, Ubuntu 14.04 x32 and Mac OS X 10.9.5.

	Sylvestre Ledru [:Sylvestre]
	Comment 29 • 9 years ago

added to the release notes "36.0.1 - Hello contacts tab did not appear after login with Firefox Account (1137141)"