Closed Bug 1359083 Opened 8 years ago Closed 8 years ago

Extract shared/css/color utils from devtools

Categories

(DevTools :: General, enhancement, P3)

Product:
DevTools
Component:
General
Type:
enhancement

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

Bug 1359083 - stop using devtools color utils in ext-browser-content.js;
59 bytes, text/x-review-board-request
kmag
: review+
Details 
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.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
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 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)
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 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 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!
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
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
https://hg.mozilla.org/mozilla-central/rev/fee49c5b98e4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: