Closed
Bug 848745
Opened 12 years ago
Closed 12 years ago
API to get CSSColor names from triplets and vice versa
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: miker, Assigned: bzbarsky)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
dbaron
:
review+
miker
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
It would be useful if you could expose the list of all color names along with their values to privileged JS. This is useful for e.g. converting rgb values to color names.
There are numerous lists of all color names and values in .js files so it would be great if we had a way to get this information directly from the browser.
Something like:
{
"red", { r: 255, g: 0, b: 0 },
"green", { r:0, g:255, b:0 },
...
}
We access the color name list in TestColorNames.cpp so it should be simple. I am just not sure how this should be exposed to JS.
Assignee | ||
Comment 1•12 years ago
|
||
I can certainly add an API on the inspector utils that returns an object like the one you describe if that's what you'd prefer.
Alternately we can do some sort of APIs that take a name and return a triple and vice versa...
What would work best for you? It seems like the "convert rgb value to color name" use case is actually a bit of a PITA given the data structure in comment 0...
Reporter | ||
Comment 2•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #1)
> I can certainly add an API on the inspector utils that returns an object
> like the one you describe if that's what you'd prefer.
>
> Alternately we can do some sort of APIs that take a name and return a triple
> and vice versa...
>
Color name to triple and vice versa sounds like it is what we would need.
Assignee | ||
Comment 3•12 years ago
|
||
OK. So take a color name and return an { r: R, g: G, b: B }? And for the triple-to-name take a triple as three arguments or a dictionary or something else?
Reporter | ||
Comment 4•12 years ago
|
||
I would say something like:
xxx.getCSSTripleToName(255, 0 ,0) returns "red"
xxx.getCSSNameTOTriple("red") returns {r: 255, g: 0, b: 0}
Assignee | ||
Comment 5•12 years ago
|
||
OK. I should be able to get this together on Monday.
Assignee: nobody → bzbarsky
Assignee | ||
Comment 6•12 years ago
|
||
Michael, I made this throw for unknown color names; let me know if you'd prefer a different behavior
Attachment #724768 -
Flags: review?(mratcliffe)
Attachment #724768 -
Flags: review?(dbaron)
Assignee | ||
Comment 7•12 years ago
|
||
Again, I made this throw for RGB triples that don't correspond to a color name
Attachment #724776 -
Flags: review?(mratcliffe)
Attachment #724776 -
Flags: review?(dbaron)
Comment 8•12 years ago
|
||
Comment on attachment 724768 [details] [diff] [review]
part 1. Add scriptable APIs for converting CSS color names to RGB triples.
Could you add a comment noting that using |octet| for rgb() color values isn't necessarily future-proof since their range can exceed 0-255, but it's ok for this usage in particular since it's only about current named colors? I just don't want this pattern to be copied somewhere it's inappropriate.
r=dbaron with that
Attachment #724768 -
Flags: review?(dbaron) → review+
Reporter | ||
Updated•12 years ago
|
Attachment #724768 -
Flags: review?(mratcliffe)
Attachment #724768 -
Flags: review?(dbaron)
Attachment #724768 -
Flags: review+
Comment 9•12 years ago
|
||
Comment on attachment 724776 [details] [diff] [review]
part 2. Add a scriptable API for converting RGB triples to CSS color names.
It seems better to export another method from nsColor.cpp/nsColor.h and use the array there rather than storing the list of color names twice.
Attachment #724776 -
Flags: review?(dbaron) → review-
Reporter | ||
Updated•12 years ago
|
Attachment #724776 -
Flags: review?(mratcliffe)
Attachment #724776 -
Flags: review?(dbaron)
Attachment #724776 -
Flags: review-
Attachment #724776 -
Flags: review+
Comment 10•12 years ago
|
||
Comment on attachment 724768 [details] [diff] [review]
part 1. Add scriptable APIs for converting CSS color names to RGB triples.
resetting review flag (See comment 8)
Attachment #724768 -
Flags: review?(dbaron) → review+
Comment 11•12 years ago
|
||
Comment on attachment 724776 [details] [diff] [review]
part 2. Add a scriptable API for converting RGB triples to CSS color names.
resetting review flag (see comment 9)
Attachment #724776 -
Flags: review?(dbaron) → review-
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #725013 -
Flags: review?(dbaron)
Assignee | ||
Updated•12 years ago
|
Attachment #724776 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
> Could you add a comment noting
Done.
Comment 14•12 years ago
|
||
Comment on attachment 725013 [details] [diff] [review]
part 2. Add a scriptable API for converting RGB triples to CSS color names.
Review of attachment 725013 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/src/nsColor.cpp
@@ +291,5 @@
> +
> +NS_GFX_(const char*)
> +NS_RGBToColorName(nscolor aColor)
> +{
> + for(size_t idx = 0; idx < ArrayLength(kColors); ++idx) {
'for ('
Assignee | ||
Comment 15•12 years ago
|
||
Attachment #725023 -
Flags: review?(dbaron)
Assignee | ||
Updated•12 years ago
|
Attachment #725013 -
Attachment is obsolete: true
Attachment #725013 -
Flags: review?(dbaron)
Comment 16•12 years ago
|
||
Comment on attachment 725023 [details] [diff] [review]
Part 2 with the space
Probably add a comment that in case of multiple matches, it returns the first one in nsColorNameList.h, which is generally the first in alphabetical order? (Assuming that's the case.)
From a US English perspective, I'm happy with that, since gray is before grey in alphabetical order. :-)
Attachment #725023 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 17•12 years ago
|
||
Done and:
https://hg.mozilla.org/integration/mozilla-inbound/rev/daa5e00d6428
https://hg.mozilla.org/integration/mozilla-inbound/rev/04e088286219
Flags: in-testsuite?
Target Milestone: --- → mozilla22
Reporter | ||
Updated•12 years ago
|
Summary: API to get all CSSColor names and values → API to get CSSColor names from triplets and vice versa
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/daa5e00d6428
https://hg.mozilla.org/mozilla-central/rev/04e088286219
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•