Closed
Bug 1316603
Opened 8 years ago
Closed 7 years ago
Add an example of extracting theme colors from an image
Categories
(WebExtensions :: General, defect, P5)
WebExtensions
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mikedeboer, Assigned: mikedeboer)
References
Details
(Whiteboard: triaged[theming])
Attachments
(1 file)
Use an image quantizer to read all the color values from an image and use the most distinctive ones ('dominant' colors) to use in the UI.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8809438 -
Flags: review?(jaws)
Comment 2•8 years ago
|
||
In case you didn't know, we already have ColorAnalyzer(_worker).js which does some similar things e.g. https://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/ColorAnalyzer_worker.js#56-66
Assignee | ||
Comment 3•8 years ago
|
||
I kinda did, but also didn't - it hadn't registered fully yet and it does exactly what we'd need here.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8809438 -
Flags: review?(jaws)
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8809438 [details] Bug 1316603 - add a findDominantColors() method to the ColorAnalyzer class to extract dominant colors from an image to use as theme colors. https://reviewboard.mozilla.org/r/92032/#review94744 ::: browser/components/extensions/ext-theme.js:513 (Diff revision 3) > // Use a temporary element to filter the CSS values that themes can provide. > if (!WindowManager.topWindow) { > return; > } > > + let deferred = false; > + > let temp = WindowManager.topWindow.document.createElement("temp"); Please keep the temp variable declaration closer to the comment about using a temporary element. ::: browser/components/extensions/ext-theme.js:543 (Diff revision 3) > } else if (image == "theme_frame" || image == "headerURL") { > this.LWTStyles.headerURL = val; > + if (!fullTheme.colors || !Object.keys(fullTheme.colors).length) { > + deferred = true; > + const CA = Cc["@mozilla.org/places/colorAnalyzer;1"].getService(Ci.mozIColorAnalyzer); > + CA.findDominantColors(Services.io.newURI(val, "", null), function(success, color, pal) { please use 'palette' instead of 'pal' here and elsewhere in the patch. ::: toolkit/components/places/ColorAnalyzer_worker.js:173 (Diff revision 3) > } > } > } > > let colors = []; > for (var color in colorFrequency) { 'color' here is a redeclaration of the variable declared at line 161, which will mean the one at line 161 will shadow this instance. Also, please use 'let' instead of 'var'. ::: toolkit/components/places/mozIColorAnalyzer.idl:28 (Diff revision 3) > * If success is false, color is not provided. > */ > - void onComplete(in boolean success, [optional] in unsigned long color); > + void onComplete(in boolean success, [optional] in unsigned long color, [optional] in jsval colors); > }; > > -[scriptable, uuid(d056186c-28a0-494e-aacc-9e433772b143)] > +[scriptable, uuid(0ce30b36-3f67-452b-a91f-0f52b04c7720)] uuids don't need to be updated anymore, per https://groups.google.com/forum/#!searchin/mozilla.dev.platform/uuid%7Csort:relevance/mozilla.dev.platform/n6qBpyxoI6I/4qxwFvBOAgAJ ::: toolkit/modules/Color.jsm:17 (Diff revision 3) > + * @param {Number} population > */ > -function Color(r, g, b) { > +function Color(r, g, b, population) { Can this comment explain what 'population' is?
Attachment #8809438 -
Flags: review?(jaws) → review+
Comment 7•8 years ago
|
||
Thanks for adding me into this bug! As far as I know there's no need to provide a new theme API for this? Just a XHR/webRequest to the favicon and a background page should be enough. I have a colour-utils module here that does the job for me, it just needs an image element: https://github.com/nt1m/vivaldi-fox/blob/master/utils/colour.js . So as long as I can access the icon, it should be fine :) Maybe it would be more helpful to have an alternative to google chrome's chrome://favicon/ uris.
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/projects/cedar/rev/e411a5414a9d2685574dd5cd5b7843e629cf21b8 Bug 1316603 - add a findDominantColors() method to the ColorAnalyzer class to extract dominant colors from an image to use as theme colors. r=jaws
Updated•7 years ago
|
Priority: -- → P5
Whiteboard: triaged → triaged[theming]
Updated•7 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•