Closed
Bug 311511
Opened 19 years ago
Closed 19 years ago
[FIX]NS_HexToRGB is slow and buggy
Categories
(Core Graveyard :: GFX, defect, P1)
Core Graveyard
GFX
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(1 file)
|
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
asa
:
approval1.8rc1-
|
Details | Diff | Splinter Review |
NS_HexToRGB uses NS_LossyConvertUTF16toASCII on the incoming string. This is
slow (shows up in profiles for bug 297959) and buggy (fails
http://web.mit.edu/bzbarsky/www/testcases/css-parsing/color-escape1.xml or the
following testcase:
data:text/html,<style>body {color: #\0166 f0000} body:before{content: "#\0166
f0000"}</style>
).
|
Assignee | |
Comment 1•19 years ago
|
||
We have only one caller of NS_ASCIIHexToRGB and lots of callers of NS_HexToRGB,
and the latter are much more perf-sensitive. Also fixes the CSS parsing bug...
Attachment #198800 -
Flags: superreview?(dbaron)
Attachment #198800 -
Flags: review?(dbaron)
|
|
Assignee | |
Comment 2•19 years ago
|
||
Oh, I was thinking this is 1.9-type stuff, since we've had this bug forever.
But if you think we should land this in 1.8, let me know.
|
||
Comment 3•19 years ago
|
||
Comment on attachment 198800 [details] [diff] [review]
Proposed fix
r+sr=dbaron
No objections to putting it on the 1.8 branch...
Attachment #198800 -
Flags: superreview?(dbaron)
Attachment #198800 -
Flags: superreview+
Attachment #198800 -
Flags: review?(dbaron)
Attachment #198800 -
Flags: review+
|
|
Assignee | |
Comment 4•19 years ago
|
||
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Comment 5•19 years ago
|
||
Comment on attachment 198800 [details] [diff] [review]
Proposed fix
I think this is worth taking on branch. While this is a somewhat long-standing
intl/perf bug, the fix is very very safe, and there's always the "IE gets this
right" aspect... ;)
Attachment #198800 -
Flags: approval1.8rc1?
|
||
Comment 6•19 years ago
|
||
could make this a template function and eliminate all conversions, if you wanted...
|
|
Assignee | |
Comment 7•19 years ago
|
||
I considered that, but the one caller of the ASCII version just isn't worth the
extra codesize. It's a very non-per-sensitive caller.
|
||
Comment 8•19 years ago
|
||
Comment on attachment 198800 [details] [diff] [review]
Proposed fix
too late for non-critical changes.
Attachment #198800 -
Flags: approval1.8rc1? → approval1.8rc1-
|
||
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•