switch computed style's url(invalid-url:) to [css3-values]'s url(about:invalid)
RESOLVED
FIXED
in mozilla19
Status
()
People
(Reporter: dbaron, Assigned: ebassi)
Tracking
Bug Flags:
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment, 1 obsolete attachment)
1.42 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
In computed style, for an invalid URL, nsROCSSPrimitiveValue.cpp sometimes generates url(invalid-url:), because we had to generate something for invalid URLs. http://dev.w3.org/csswg/css3-values/ describes url(about:invalid) for a similar purpose, so we should switch this to being about:invalid instead.
(Assignee) | ||
Comment 1•7 years ago
|
||
Created attachment 659681 [details] [diff] [review] Trivial patch switching from url(invalid-url:) to url(about:invalid)
Attachment #659681 -
Flags: review?(dbaron)
(Reporter) | ||
Comment 2•7 years ago
|
||
Comment on attachment 659681 [details] [diff] [review] Trivial patch switching from url(invalid-url:) to url(about:invalid) Perhaps add a comment saying something like: // http://dev.w3.org/csswg/css3-values/#attr-notation defines // about:invalid for a similar purpose, so let's also use it // here as the computed value for invalid URLs. Though I wonder if it's an issue that this *does* round trip to something meaningful. (Might we need to make about:invalid produce a particular empty document?)
![]() |
||
Comment 3•7 years ago
|
||
Spec says: The about:invalid URI references a non-existent document with a generic error condition. It can be used when a URI is necessary, but the default value shouldn't be resolveable as any type of document. So I think the answer to the last part of comment 2 is "no".
(Assignee) | ||
Comment 4•7 years ago
|
||
Created attachment 660097 [details] [diff] [review] new patch, updated after review
Attachment #659681 -
Attachment is obsolete: true
Attachment #659681 -
Flags: review?(dbaron)
Attachment #660097 -
Flags: review?(dbaron)
Updated•7 years ago
|
Assignee: nobody → ebassi
(Reporter) | ||
Comment 5•6 years ago
|
||
Comment on attachment 660097 [details] [diff] [review] new patch, updated after review When I asked if it was an issue, I was more worried about "for the Web" than "according to the spec". But I suppose let's go ahead and do this, even though it's a little weird.
Attachment #660097 -
Flags: review?(dbaron) → review+
(Assignee) | ||
Comment 6•6 years ago
|
||
setting the checkin-needed keyword as I don't have the level 2 commit bit yet
Keywords: checkin-needed
Comment 7•6 years ago
|
||
I don't see a Try link here, so I've pushed it myself. I'll land this if it's green. https://tbpl.mozilla.org/?tree=Try&rev=e29c3fd60ea9
Comment 8•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c01e44583414 Should this have a test?
Flags: in-testsuite?
Keywords: checkin-needed
Comment 9•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c01e44583414
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in
before you can comment on or make changes to this bug.
Description
•