Closed Bug 1328756 Opened 8 years ago Closed 8 years ago

Add a pref for and options to the Long Press of the plus button

Categories

(Firefox :: Tabbed Browser, defect, P1)

Product:
Firefox
Component:
Tabbed Browser
Version:
52 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: tanvi, Assigned: baku)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId])

Attachments

(1 file)

click.patch
9.87 KB, patch
Gijs
: review+
Details | Diff | Splinter Review
Add a pref for the Long Press of the plus button - https://bugzilla.mozilla.org/show_bug.cgi?id=1272256 - with multiple options 1) Long press pulls up Containers menu 2) First click pulls up Containers menu 3) Long press does nothing Marking as P1 for now, and may move to P2 or P3 after discussion in tomorrows Containers/TestPilot meeting.
Assignee: nobody → amarchesini
This for 52 and 53.
Attached patch click.patch — — Splinter Review
Attachment #8824401 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8824401 [details] [diff] [review] click.patch Review of attachment 8824401 [details] [diff] [review]: ----------------------------------------------------------------- r=me if we can get rid of the setTimeout(..., 2000). Otherwise, please explain and re-request review. :-) ::: browser/base/content/tabbrowser.xml @@ +5482,5 @@ > this._browserNewtabpageEnabled = Services.prefs.getBoolPref("browser.newtabpage.enabled"); > this.observe(null, "nsPref:changed", "privacy.userContext.enabled"); > Services.prefs.addObserver("privacy.userContext.enabled", this, false); > + this.observe(null, "nsPref:changed", "privacy.usercontext.longPressBehavior"); > + Services.prefs.addObserver("privacy.usercontext.longPressBehavior", this, false); pref observers are substring-at-first-index ("starts with") matching, so we could just use one observer? @@ +5525,5 @@ > <parameter name="aData"/> > <body><![CDATA[ > switch (aTopic) { > case "nsPref:changed": > // This is the only pref observed. This comment is no longer accurate either way. Can we update this comment, and perhaps the code if we do go for a single pref observer? @@ +5530,5 @@ > let containersEnabled = Services.prefs.getBoolPref("privacy.userContext.enabled") > && !PrivateBrowsingUtils.isWindowPrivate(window); > > + // This pref is not meant to change so often. Recreating the menu > + // here is somehow acceptable. "somehow" is a bit odd in this context, I think, so I would go for something like: // This pref won't change so often, so just recreate the menu. ::: browser/components/contextualidentity/test/browser/browser_newtabButton.js @@ +76,5 @@ > + let newTab = document.getElementById('tabbrowser-tabs'); > + let newTabButton = document.getAnonymousElementByAttribute(newTab, "anonid", "tabs-newtab-button"); > + ok(newTabButton, "New tab button exists"); > + ok(!newTabButton.hidden, "New tab button is visible"); > + yield new Promise((resolve, reject) => { setTimeout(resolve, 2000); }); We shouldn't need to wait at all, and hardcoded timeouts like this are very evil. Why do we need it here?
Attachment #8824401 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d80f960a1f2d Add a pref for and options to the Long Press of the plus button, r=gijs
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/305dcbada31f Remove pref observer privacy.userContext, r=me
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f248d089469d Add a pref for and options to the Long Press of the plus button, r=gijs
https://hg.mozilla.org/mozilla-central/rev/f248d089469d
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: