Closed
Bug 1359083
Opened 8 years ago
Closed 8 years ago
Extract shared/css/color utils from devtools
Categories
(DevTools :: General, enhancement, P3)
DevTools
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
Attachments
(1 file)
toolkit/components/extensions/ext-browser-content.js relies on devtools shared/css/color utils to check if a given color is opaque or not : http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/toolkit/components/extensions/ext-browser-content.js#215-220 We should either extract shared/css/color so that it stays in mozilla-central, or implement a simpler workaround.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8861137 [details] Bug 1359083 - stop using devtools color utils in ext-browser-content.js; Kris: As explained in the summary, we are moving DevTools out of m-c and I'm trying to cleanup the dependencies on DevTools from the rest of the codebase. WebExtensions rely on devtools color utils to check if a given color is opaque. What do you think about this patch, simply creating a local dedicated method to do just that? Looking at the dependencies, extracting color.js from devtools actually means extracting the whole devtools/shared/css folder, so I would like to avoid it if possible.
Attachment #8861137 -
Flags: feedback?(kmaglione+bmo)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8861137 [details] Bug 1359083 - stop using devtools color utils in ext-browser-content.js; (re-flagging for feedback, see comment 3)
Attachment #8861137 -
Flags: feedback?(kmaglione+bmo)
Comment 6•8 years ago
|
||
Triaging dt-addon blockers as P3/enhancement since we're not using priorities for this project (filter on CLIMBING SHOES).
Severity: normal → enhancement
Priority: -- → P3
Comment hidden (mozreview-request) |
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8861137 [details] Bug 1359083 - stop using devtools color utils in ext-browser-content.js; https://reviewboard.mozilla.org/r/133088/#review137794 Thanks! ::: toolkit/components/extensions/ext-browser-content.js:59 (Diff revision 4) > + > + return true; > + } catch (e) { > + // Invalid color. > + return true; > + } Nit: Please just use an empty catch block, and one `return true` after the try-catch.
Attachment #8861137 -
Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8861137 [details] Bug 1359083 - stop using devtools color utils in ext-browser-content.js; https://reviewboard.mozilla.org/r/133088/#review137794 > Nit: Please just use an empty catch block, and one `return true` after the try-catch. done!
Assignee | ||
Comment 11•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8861137 [details] Bug 1359083 - stop using devtools color utils in ext-browser-content.js; https://reviewboard.mozilla.org/r/133088/#review137794 Thanks for the review! New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=055f00cf7bd0dec382613278437d8c563a324658
Comment 12•8 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fee49c5b98e4 stop using devtools color utils in ext-browser-content.js;r=kmag
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fee49c5b98e4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•