Closed
Bug 1313459
Opened 8 years ago
Closed 7 years ago
Extend browser_action to allow putting buttons in other CUI areas
Categories
(WebExtensions :: Frontend, defect, P3)
WebExtensions
Frontend
Tracking
(firefox54 fixed)
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.
Updated•8 years ago
|
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Whiteboard: [design-decision-needed]
Comment 1•7 years ago
|
||
To be discussed at Jan 10 WebExtensions Triage meeting. Agenda: https://docs.google.com/document/d/18K97o1juaHSeYEkes1iMz8AayjuVkUuIK844ErGaa-c/edit#
Reporter | ||
Comment 2•7 years ago
|
||
To be more clear, the desired result would be the same as: CustomizableUI.createWidget({..., defaultArea: CustomizableUI.AREA_PANEL}
Assignee | ||
Comment 3•7 years ago
|
||
@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
Comment 4•7 years ago
|
||
(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)
Assignee | ||
Comment 5•7 years ago
|
||
Yes, default would be toolbar.
Assignee | ||
Updated•7 years ago
|
webextensions: --- → ?
Priority: P5 → P3
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → mixedpuppy
Assignee | ||
Updated•7 years ago
|
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 hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
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 8•7 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
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.
Assignee | ||
Updated•7 years ago
|
Attachment #8838647 -
Flags: feedback?(gijskruitbosch+bugs)
Comment 11•7 years ago
|
||
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+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: dev-doc-needed
Whiteboard: [design-decision-approved] triaged → triaged
Comment 13•7 years ago
|
||
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3d9ddc7244ad support CUI areas for browserAction, r=aswan
Comment 14•7 years ago
|
||
sorry had to back this out for browser chrome failures like https://treeherder.mozilla.org/logviewer.html#?job_id=78894877&repo=autoland&lineNumber=1903
Flags: needinfo?(mixedpuppy)
Comment 15•7 years ago
|
||
Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1fc93c32ef2e Backed out changeset 3d9ddc7244ad for bunch of browser chrome failures
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
> ::: 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)
Comment 18•7 years ago
|
||
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/14dbbe36e676 support CUI areas for browserAction, r=aswan
Comment 19•7 years ago
|
||
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.
Comment 20•7 years ago
|
||
The latter, https://treeherder.mozilla.org/logviewer.html#?job_id=79227200&repo=autoland is Win8.
Assignee | ||
Comment 21•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5063a1e2bab
Assignee | ||
Comment 22•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=40b99e2fc0a6
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•7 years ago
|
||
third times a charm (x fingers). But actually, the last try did work out.
Comment 25•7 years ago
|
||
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c2c5efc2b575 support CUI areas for browserAction, r=aswan
Comment 26•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c2c5efc2b575
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 27•7 years ago
|
||
(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)
Comment 28•7 years ago
|
||
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)
Updated•7 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•