Closed Bug 1313459 Opened 8 years ago Closed 8 years ago

Extend browser_action to allow putting buttons in other CUI areas

Categories

(WebExtensions :: Frontend, defect, P3)

defect

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed
webextensions ?

People

(Reporter: miketaylr, Assigned: mixedpuppy)

References

Details

(Keywords: dev-doc-complete, Whiteboard: triaged)

Attachments

(1 file)

As a user I'd like an option to put my web extension button in the panel by default. Right now the only way to do this is with CustomizableUI.jsm.
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Whiteboard: [design-decision-needed]
To be discussed at Jan 10 WebExtensions Triage meeting. Agenda: https://docs.google.com/document/d/18K97o1juaHSeYEkes1iMz8AayjuVkUuIK844ErGaa-c/edit#
To be more clear, the desired result would be the same as: CustomizableUI.createWidget({..., defaultArea: CustomizableUI.AREA_PANEL}
@gijs: we feel it's a good idea to at least allow the panel as an option. I was thinking that it may be nice to allow other area's as options as well, but we wanted to get your input on that. good idea? bad idea? other?
Flags: needinfo?(gijskruitbosch+bugs)
Priority: -- → P5
Whiteboard: [design-decision-needed] → [design-decision-approved] triaged
(In reply to Shane Caraveo (:mixedpuppy) from comment #3) > @gijs: we feel it's a good idea to at least allow the panel as an option. I > was thinking that it may be nice to allow other area's as options as well, > but we wanted to get your input on that. good idea? bad idea? other? Given that other add-ons can do this I don't see any reason webextensions shouldn't be able to. I assume right now we default to the toolbar?
Flags: needinfo?(gijskruitbosch+bugs)
Yes, default would be toolbar.
webextensions: --- → ?
Priority: P5 → P3
Assignee: nobody → mixedpuppy
Summary: Extend browser_action to allow putting buttons in hamburger panel by default → Extend browser_action to allow putting buttons in other CUI areas
Comment on attachment 8838647 [details] Bug 1313459 support CUI areas for browserAction, Also asking Gijs for feedback from a CUI perspective.
Attachment #8838647 - Flags: feedback?(gijskruitbosch+bugs)
Comment on attachment 8838647 [details] Bug 1313459 support CUI areas for browserAction, https://reviewboard.mozilla.org/r/113482/#review115090 The webextensions bits look good, but somebody familiar with CustomizableUI should also have a glance. I see you've got an f? to Gijs for that... ::: browser/components/extensions/ext-browserAction.js:43 (Diff revision 1) > } > > // WeakMap[Extension -> BrowserAction] > var browserActionMap = new WeakMap(); > > +var browserAreas = { const ::: browser/components/extensions/ext-browserAction.js:72 (Diff revision 1) > title: options.default_title || extension.name, > badgeText: "", > badgeBackgroundColor: null, > icon: IconDetails.normalize({path: options.default_icon}, extension), > popup: options.default_popup || "", > + area: options.default_area && browserAreas[options.default_area], If you add `"default": "navbar"` to the schema, then you don't need any conditionals here and below where you set the default.
Attachment #8838647 - Flags: review?(aswan) → review+
Comment on attachment 8838647 [details] Bug 1313459 support CUI areas for browserAction, https://reviewboard.mozilla.org/r/113482/#review115090 Yeah, this can wait for Gijs.
Attachment #8838647 - Flags: feedback?(gijskruitbosch+bugs)
Comment on attachment 8838647 [details] Bug 1313459 support CUI areas for browserAction, Sorry for the delay. Yeah, this looks OK to me, though it would be a good idea to add a test that the default (when no area is specified) is the nav-bar, maybe? If we think that's already covered by other tests then a comment to that effect might be helpful. The other thing I feel like I should say is that apparently there are some photon designs (still in flux, as I understand it) that show us altering the customizable area concepts again. Without stating any opinions about that idea, I would keep that in mind when announcing / publishing / stabilizing this API. In other words, how would we deal with extra areas / some areas going away? I'm not that familiar with the internals of how we handle options, but migrating / interpreting option A as equivalent to option B might need to be possible in the future, if it isn't yet.
Attachment #8838647 - Flags: feedback?(gijskruitbosch+bugs) → feedback+
Keywords: dev-doc-needed
Whiteboard: [design-decision-approved] triaged → triaged
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3d9ddc7244ad support CUI areas for browserAction, r=aswan
Flags: needinfo?(mixedpuppy)
Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1fc93c32ef2e Backed out changeset 3d9ddc7244ad for bunch of browser chrome failures
> ::: browser/components/extensions/ext-browserAction.js:72 > (Diff revision 1) > > title: options.default_title || extension.name, > > badgeText: "", > > badgeBackgroundColor: null, > > icon: IconDetails.normalize({path: options.default_icon}, extension), > > popup: options.default_popup || "", > > + area: options.default_area && browserAreas[options.default_area], > > If you add `"default": "navbar"` to the schema, then you don't need any > conditionals here and below where you set the default. This didn't actually work, and added back default value into the code. However I had left the default value in schema, and while it didn't choke in my testing, it did when it landed with an invalid schema value. I don't see default in any other schema as well.
Flags: needinfo?(mixedpuppy) → needinfo?(aswan)
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/14dbbe36e676 support CUI areas for browserAction, r=aswan
Backed out in https://hg.mozilla.org/integration/autoland/rev/e52cd3f75ebf for https://treeherder.mozilla.org/logviewer.html#?job_id=79222095&repo=autoland - might be down to just Win7 opt timeouts, or we might just be horribly slow about getting around to running it on enough other platforms, dunno.
third times a charm (x fingers). But actually, the last try did work out.
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c2c5efc2b575 support CUI areas for browserAction, r=aswan
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(In reply to Shane Caraveo (:mixedpuppy) from comment #17) > This didn't actually work, and added back default value into the code. > However I had left the default value in schema, and while it didn't choke in > my testing, it did when it landed with an invalid schema value. sorry, i'm having trouble parsing the above... what's the question? > I don't see > default in any other schema as well. its not widely used but eg http://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/browser/components/extensions/schemas/sessions.json#68
Flags: needinfo?(aswan)
Depends on: 1344408
Depends on: 1354109
I've added a bit here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/browser_action that talks about "default_area" but let me know it we need anything else. Thanks!
Flags: needinfo?(mixedpuppy)
Docs LGTM
Flags: needinfo?(mixedpuppy)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: