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)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.1 - Jun 26
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.
Flags: qe-verify?
Priority: -- → P2
Component: General → Toolbars and Customization
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Priority: P2 → P1
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 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)
(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 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+
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+
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. :-\
(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.
Is Comment 10 a correct summary of the situation, and do you have an opinion?
Flags: needinfo?(gijskruitbosch+bugs)
(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 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+
Blocks: 1375080
Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c4c815e1ad5 Add the sidebar button into the default toolbar set;r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Iteration: --- → 56.1 - Jun 26
"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]
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1387081
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: