895921 - [MP] Defect - Tab bar not appearing when selecting links from flyouts

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Flyouts, defect, P2)

Description

[Kamil Jozwiak [:kjozwiak]]

When selecting "Help (online)", the "Tabs App Bar" isn't appearing making it seam like a new tab hasn't been created.

Steps to reproduce the issue:

1) Open Firefox Metro
2) Swipe in the Windows Charm flyout and select "Settings"
3) Select "Help (online)" and you'll notice that the "Tabs App Bar" will not slide in indicating a new tab has been opened

Current Behavior:

- Selecting "Help (online)" will not slide in the "Tabs" app bar indicating that a new tab has been created

Expected Behavior:

- When a user selects "Help (online)" from the "Settings" flyout, the "Tabs" app bar should slide in showing the user that a new tab has been created

Comment 1

[Kamil Jozwiak [:kjozwiak]]

Selecting "Read the Nightly privacy policy online" from the "About" flyout has the same issue.

Comment 2

[Allison Naaktgeboren :ally]

the privacy policy handler calls a function that has Browser.addTab()'s aBringFront hardcoded to true..this might be more tricky than we thought in triage, but I won't know until I can get a build standing again.

Comment 3

[Jim Mathies [:jimm]]

(In reply to :Ally Naaktgeboren from comment #2)
> the privacy policy handler calls a function that has Browser.addTab()'s
> aBringFront hardcoded to true..this might be more tricky than we thought in
> triage, but I won't know until I can get a build standing again.

I think all that's needed is an added call to peekTabs.

http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/ContextUI.js#151

Comment 4

[Matt Brubeck (:mbrubeck)]

(In reply to :Ally Naaktgeboren from comment #2)
> the privacy policy handler calls a function that has Browser.addTab()'s
> aBringFront hardcoded to true.

I think we still want the new tab to appear in the foreground, so this should not be a problem.  (But if we did want it to appear in the background, we could just switch to calling Browser.addTab(url, false).)

Comment 5

[Allison Naaktgeboren :ally]

Attachment: bringing in the tabstrip

When I picked up the bug, the newtab was not foregrounded and the tabstrip was not sliding in. The newtab was foregrounded at some point (yay), and this brings the tabstrip in.

That said, I wonder whether I should refactor this code so that BrowserUI also controls the privacy policy link. (I don't think it really belongs in browser.js). It won't make a functional difference, but I think it will make a maintainability/code sanity difference. Worthwhile, or am I over rotating?

Comment 6

[Matt Brubeck (:mbrubeck)]

Comment on attachment 786620 [details] [diff] [review]
bringing in the tabstrip

I agree; onAboutPolicyClick should move out of browser.js, maybe into AboutFlyoutPanel or BrowserUI.  We should use the same constant as the argument to peekTabs in both of these calls -- maybe even move the peekTabs call inside of BrowserUI.newTab.

Comment 7

[Allison Naaktgeboren :ally]

Attachment: peektabs

I feel better much better. :)

turns out that peekTabs() isn't called near newTab as often as I thought. I left most of the incantations of newTab() alone since the 3rd parameter defaults to false.

Comment 8

[Allison Naaktgeboren :ally]

Attachment: peektabs

Comment 9

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/integration/fx-team/rev/85c1aed7e525

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/85c1aed7e525

Comment 11

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #12 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-08-14-03-02-04-mozilla-central/

- Went through the original test case in comment #0 without any issues (went through it 15 different times)
- Ensured all the flyouts where still working without any issues (sliding into view)
- Ensured that you can go back and forth between the different flyouts without any issues
- Ensured that all the flyouts where formatted and appear correctly
- Ensured that both links from "Help" & "About" had the same behavior (when opening into a new tab)
- Ensured that all of the above cases also worked using Filled View

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #13 without any issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-08-31-03-02-24-mozilla-central/

- Went through the original test case in comment #0 without any issues
- Went through the test cases added in comment #11 without any issues

Comment 13

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0

Tested this on latest nightly (build ID: 20131030030201) during iteration #17 testing.
No issues encountered, marking as verified.