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)
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•8 years ago
|
||
To be discussed at Jan 10 WebExtensions Triage meeting.
Agenda: https://docs.google.com/document/d/18K97o1juaHSeYEkes1iMz8AayjuVkUuIK844ErGaa-c/edit#
Reporter | ||
Comment 2•8 years ago
|
||
To be more clear, the desired result would be the same as:
CustomizableUI.createWidget({..., defaultArea: CustomizableUI.AREA_PANEL}
Assignee | ||
Comment 3•8 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•8 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•8 years ago
|
||
Yes, default would be toolbar.
Assignee | ||
Updated•8 years ago
|
webextensions: --- → ?
Priority: P5 → P3
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mixedpuppy
Assignee | ||
Updated•8 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•8 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•8 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•8 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•8 years ago
|
Attachment #8838647 -
Flags: feedback?(gijskruitbosch+bugs)
Comment 11•8 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•8 years ago
|
Keywords: dev-doc-needed
Whiteboard: [design-decision-approved] triaged → triaged
Comment 13•8 years ago
|
||
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3d9ddc7244ad
support CUI areas for browserAction, r=aswan
Comment 14•8 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•8 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•8 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•8 years ago
|
||
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/14dbbe36e676
support CUI areas for browserAction, r=aswan
Comment 19•8 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•8 years ago
|
||
The latter, https://treeherder.mozilla.org/logviewer.html#?job_id=79227200&repo=autoland is Win8.
Assignee | ||
Comment 21•8 years ago
|
||
Assignee | ||
Comment 22•8 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•8 years ago
|
||
third times a charm (x fingers). But actually, the last try did work out.
Comment 25•8 years ago
|
||
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c2c5efc2b575
support CUI areas for browserAction, r=aswan
Comment 26•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 27•8 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•8 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•8 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Updated•7 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•