Closed
Bug 1279143
Opened 9 years ago
Closed 9 years ago
color indicator missing when "File -> New Container Tab" without any windows opened
Categories
(Firefox :: Tabbed Browser, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 50
People
(Reporter: kjozwiak, Assigned: baku)
References
(Blocks 1 open bug)
Details
(Whiteboard: [usercontextId], [domsecurity-active][uplift49+])
Attachments
(2 files, 1 obsolete file)
42.87 KB,
image/png
|
Details | |
3.05 KB,
patch
|
Gijs
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
When opening a container via "File -> New Container Tab" under OSX while there's no windows currently opened, the colour indicator doesn't appear above the tab. The UI in the awesomebar is correctly displayed, but the colour indicator on top of the tab is missing.
* attached a screenshot to illustrate the issue
STR:
* open m-c and ensure containers are enabled via "privacy.userContext.enabled;true"
* close all the windows currently opened
* select File -> New Container Tab -> Personal Container
You'll notice that the UI under the awesomebar is correctly being displayed, but the colour indicator at the top of the tab is missing.
Build Used:
* fx50.0a1, buildID: 20160608030219, changeset: f8ad071a6e14
* https://archive.mozilla.org/pub/firefox/nightly/2016/06/2016-06-08-03-02-19-mozilla-central/
Updated•9 years ago
|
Priority: -- → P1
Comment 1•9 years ago
|
||
This one is also kind of a big deal, but at least it is probably OSX specific and can only occur when no windows were originally opened. This could easily confuse a user about what container they are in.
Assignee | ||
Comment 2•9 years ago
|
||
We have to style the tab manually all the time we set usercontextid.
Assignee: nobody → amarchesini
Attachment #8761484 -
Flags: review?(gijskruitbosch+bugs)
Updated•9 years ago
|
Whiteboard: [userContextId] → [usercontextId], [domsecurity-active]
Comment 3•9 years ago
|
||
Comment on attachment 8761484 [details] [diff] [review]
tab.patch
Review of attachment 8761484 [details] [diff] [review]:
-----------------------------------------------------------------
Can you refactor the two other places where ContextualIdentityService.setTabStyle is currently called to call tab.setUserContextId() as well, to reduce code duplication? It may be necessary to teach this method to deal with a not-yet-present linkedBrowser.
Attachment #8761484 -
Flags: review?(gijskruitbosch+bugs)
Comment 4•9 years ago
|
||
In fact, with the styling being so orthogonal to the attribute on the tab, and the <browser> elements having their own property, is it actually necessary to keep the usercontextid on the tab in addition to keeping it on the browser? It seems superfluous.
Assignee | ||
Comment 5•9 years ago
|
||
We still need the usercontextid attribute in the tab, but this patch centralize the calling of setuserContextId.
Attachment #8761484 -
Attachment is obsolete: true
Attachment #8761689 -
Flags: review?(gijskruitbosch+bugs)
Updated•9 years ago
|
Attachment #8761689 -
Flags: review?(gijskruitbosch+bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80ca88ff3e6a
Styling of tabs should happen all the time the attribute usercontextid is set, r=gijs
Comment 7•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Updated•9 years ago
|
Component: DOM: Security → Tabbed Browser
Product: Core → Firefox
Target Milestone: mozilla50 → ---
Updated•9 years ago
|
Target Milestone: --- → Firefox 50
Need uplift?
Whiteboard: [usercontextId], [domsecurity-active] → [usercontextId], [domsecurity-active][uplift49+]
Reporter | ||
Comment 9•9 years ago
|
||
I opened each container via the file menu when there's no windows opened and ensured that:
* the gfx indicator at the top of the tabs is being displayed correctly
* the gfx indicator under the awesomebar is being displayed correctly
* OSX 10.11.5 x64 VM - PASSED
Used the following build for verification:
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-06-03-02-33-mozilla-central/
Comment on attachment 8761689 [details] [diff] [review]
tab.patch
Approval Request Comment
[Feature/regressing bug #]:Testpilot for containers
[User impact if declined]: UI will not function in certain situation with containers test-pilot.
[Describe test coverage new/current, TreeHerder]: Some Origin Attributes testing is covered in CAPS unit tests, more is in progress.
[Risks and why]: This involves platform changes that cannot be reprecated by an addon. In order to do a Test Pilot experiment for Containers in September / Firefox 49 release, we need this code in release. Without it, testing is delayed until November and getting the capabilities of the feature to users is delayed even further.
[String/UUID change made/needed]: None
Attachment #8761689 -
Flags: approval-mozilla-aurora?
(In reply to Paul Theriault [:pauljt] from comment #10)
> Comment on attachment 8761689 [details] [diff] [review]
> tab.patch
>
> Approval Request Comment
> [Feature/regressing bug #]:Testpilot for containers
> [User impact if declined]: UI will not function in certain situation with
> containers test-pilot.
> [Describe test coverage new/current, TreeHerder]: Some Origin Attributes
> testing is covered in CAPS unit tests, more is in progress.
> [Risks and why]: This involves platform changes that cannot be reprecated by
> an addon. In order to do a Test Pilot experiment for Containers in
> September / Firefox 49 release, we need this code in release. Without it,
> testing is delayed until November and getting the capabilities of the
> feature to users is delayed even further.
> [String/UUID change made/needed]: None
Commenting further to add risk details as requested via email. This is a simple fix that will correct a known issue with the Test Pilot version of containers. This change will only affect test pilot users (not regular users who dont have the containers pref enabled).
Updated•9 years ago
|
status-firefox49:
--- → affected
Comment 12•9 years ago
|
||
Comment on attachment 8761689 [details] [diff] [review]
tab.patch
Needed for the Test Pilot container/identity feature, taking it.
Attachment #8761689 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Flags: qe-verify+
Comment 13•9 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 14•9 years ago
|
||
Went through verification using the following builds:
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-28-00-40-10-mozilla-aurora/
** fx49.0a2, buildId: 20160728004010, changeset: 5d074cab0660
Test Cases Used:
* went through the original STR from comment#0 without any issues
* went through the test cases from comment# without any issues
* ensured that all the UI indicators are present when going through
** File -> New Container Tab
** Right Click Context Menu -> Open Link In New Container Tab
** Hamburger Menu -> Open Container Tab
** Down Arrow (when there's a lot of tabs opened) -> New Container Tab
Reporter | ||
Comment 15•9 years ago
|
||
Went through verification using the following build:
* https://archive.mozilla.org/pub/firefox/nightly/2016/07/2016-07-31-03-02-03-mozilla-central/
** fx50.0a1, buildId: 20160731030203, changeset: e5859dfe0bcb
* https://archive.mozilla.org/pub/firefox/nightly/2016/08/2016-08-02-03-04-37-mozilla-central/
** fx51.0a1, buildId: 20160802030437, changeset: ffac2798999c
Went through the test cases from comment#0, comment#9 and comment#14 without any issues.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•