Closed
Bug 1364238
Opened 8 years ago
Closed 7 years ago
Add the sidebar button into the default toolbar set
Categories
(Firefox :: Toolbars and Customization, enhancement, P1)
Firefox
Toolbars and Customization
Tracking
()
Tracking | Status | |
---|---|---|
firefox56 | --- | verified |
People
(Reporter: bgrins, Assigned: bgrins)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-structure])
Attachments
(1 file)
As per https://mozilla.invisionapp.com/share/XRBHN5SWD#/screens, the sidebar button will appear by default in the toolbar.
Updated•8 years ago
|
Flags: qe-verify?
Priority: -- → P2
Updated•8 years ago
|
Component: General → Toolbars and Customization
Flags: qe-verify? → qe-verify+
Updated•8 years ago
|
QA Contact: gwimberly
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Updated•7 years ago
|
Priority: P2 → P1
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
I feel like this is missing something but I'm not sure what. I don't see any related failures on a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=921701d08c17de43f3b4edb9a43fe2129e46cf37&selectedJob=106828662
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8877749 [details]
Bug 1364238 - Add the sidebar button into the default toolbar set;
https://reviewboard.mozilla.org/r/149178/#review153916
I think you also need to update BrowserUITelemetry. Which looks like it already missed an update for the social share button, and maybe other stuff? Maybe we should put some effort in to make it rely on default placements as reported by CUI directly instead of boilerplating all the things... doesn't need to happen in this bug though.
Attachment #8877749 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to :Gijs from comment #3)
> Comment on attachment 8877749 [details]
> Bug 1364238 - Add the sidebar button into the default toolbar set;
>
> https://reviewboard.mozilla.org/r/149178/#review153916
>
> I think you also need to update BrowserUITelemetry. Which looks like it
> already missed an update for the social share button, and maybe other stuff?
> Maybe we should put some effort in to make it rely on default placements as
> reported by CUI directly instead of boilerplating all the things... doesn't
> need to happen in this bug though.
Filed Bug 1374709 for that
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8877749 [details]
Bug 1364238 - Add the sidebar button into the default toolbar set;
https://reviewboard.mozilla.org/r/149178/#review155768
Assuming this is green on try, r=me
Attachment #8877749 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8877749 [details]
Bug 1364238 - Add the sidebar button into the default toolbar set;
https://reviewboard.mozilla.org/r/149178/#review155770
Ah, oh, so, one more thing... presumably we want to add this to the navbar for existing users? This patch doesn't do that yet. Sorry, I should have thought about this in my first review. :-\
Attachment #8877749 -
Flags: review+
Comment 9•7 years ago
|
||
Though maybe we want to postpone doing that until 57? Or something? I dunno. We don't have good mechanisms for doing this conditionally on nightly now but not incrementing some kind of count that would then be out-of-sync when we take it out from MOZ_PHOTON_THEME in the 57 cycle. :-\
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to :Gijs from comment #9)
> Though maybe we want to postpone doing that until 57? Or something? I dunno.
> We don't have good mechanisms for doing this conditionally on nightly now
> but not incrementing some kind of count that would then be out-of-sync when
> we take it out from MOZ_PHOTON_THEME in the 57 cycle. :-\
So the problem is that we may not want to migrate users immediately because AppConstants.MOZ_PHOTON_THEME will still be off by default in Beta 56? And we could do a migration only if AppConstants.MOZ_PHOTON_THEME is true so it won't accidentally migrate Beta users to have the sidebar button by default, but also means that when we do flip that flag those same users *won't* get the migration because they've already had the chance and missed it.
So we need to either not do a migration now and instead wait until 57 (which means current nightly users won't get the change) or do the migration now conditionally *and* do it again in 57 (which means current nightly users will get the change, although if they then remove the sidebar it might pop back up again with the new migration).
I'm inclined to file a new bug for the migration and land it at the beginning of 57 to avoid doing it twice.
Assignee | ||
Comment 11•7 years ago
|
||
Is Comment 10 a correct summary of the situation, and do you have an opinion?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 12•7 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #10)
> I'm inclined to file a new bug for the migration and land it at the
> beginning of 57 to avoid doing it twice.
Yeah, that would be the simplest.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8877749 [details]
Bug 1364238 - Add the sidebar button into the default toolbar set;
https://reviewboard.mozilla.org/r/149178/#review156084
Attachment #8877749 -
Flags: review+
Comment 14•7 years ago
|
||
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c4c815e1ad5
Add the sidebar button into the default toolbar set;r=Gijs
Comment 15•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Iteration: --- → 56.1 - Jun 26
Comment 16•7 years ago
|
||
"Sidebar button into the default toolbar set" feature has been implemented with Latest Nightly 56.0a1 on Windows 8.1 (64 bit).
Build ID : 20170623045418
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170621]
Comment 17•7 years ago
|
||
Verified on Windows, Mac, and Ubuntu.
You need to log in
before you can comment on or make changes to this bug.
Description
•