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

VERIFIED FIXED in Firefox 26

Status

defect
P2
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: kjozwiak, Assigned: ally)

Tracking

unspecified
Firefox 26
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
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
(Reporter)

Comment 1

6 years ago
Selecting "Read the Nightly privacy policy online" from the "About" flyout has the same issue.
(Reporter)

Updated

6 years ago
Summary: Defect - Tab bar not appearing when selecting "Help (online)" → Defect - Tab bar not appearing when selecting links from flyouts
(Assignee)

Updated

6 years ago
Assignee: nobody → ally
Assignee: ally → nobody
Priority: -- → P2
(Assignee)

Updated

6 years ago
Assignee: nobody → ally
(Assignee)

Comment 2

6 years ago
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.
(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
(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).)

Updated

6 years ago
Whiteboard: feature=defect status=for_sprint c=Settings_pane_options_and_about u=metro_firefox_user p=0 → feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 status=for_sprint
Blocks: metrov1it12
No longer blocks: metrov1defect&change
Status: NEW → ASSIGNED
QA Contact: jbecerra
Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 status=for_sprint → feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3
(Assignee)

Updated

6 years ago
Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3 → feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3 [preview]
(Assignee)

Comment 5

6 years ago
Posted patch bringing in the tabstrip (obsolete) — Splinter Review
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?
Attachment #786620 - Flags: feedback?(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.
Attachment #786620 - Flags: feedback?(mbrubeck) → feedback+
(Assignee)

Comment 7

6 years ago
Posted patch peektabs (obsolete) — Splinter Review
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.
Attachment #786620 - Attachment is obsolete: true
Attachment #786686 - Flags: review?(mbrubeck)
(Assignee)

Comment 8

6 years ago
Posted patch peektabsSplinter Review
Attachment #786686 - Attachment is obsolete: true
Attachment #786686 - Flags: review?(mbrubeck)
Attachment #786690 - Flags: review?(mbrubeck)
Attachment #786690 - Flags: review?(mbrubeck) → review+
https://hg.mozilla.org/mozilla-central/rev/85c1aed7e525
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
(Reporter)

Comment 11

6 years ago
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
Summary: Defect - Tab bar not appearing when selecting links from flyouts → [MP] Defect - Tab bar not appearing when selecting links from flyouts
Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3 [preview] → [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3
(Reporter)

Comment 12

6 years ago
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
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.
Status: RESOLVED → VERIFIED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.