Closed
Bug 70734
Opened 24 years ago
Closed 24 years ago
[CSS3] RFE implement the three new CSS3 attribute selector
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
People
(Reporter: glazou, Assigned: glazou)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
The CSS 3 Selectors module reaching these days CR status, it becomes reasonable
to start implementing the most useful new selectors it introduces.
The new [attr ^= value]
[attr $= value]
[attr *= value] selectors are easy to implement and can be very useful.
For exemple, the last one could help filtering ads on a website just checking if
the SRC attribute contains a given domain...
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
proposed implementation ; tested with CSS 3 test suite ==> green :-)))
r= needed
Comment 3•24 years ago
|
||
Review keyword, cc self. Looking forward to seeing CSS3 as a full rec.
Keywords: review
Comment 4•24 years ago
|
||
Are there any switch statements where you see warnings because you need to
check for the new tokens? (Perhaps the only one was in TestCSSScanner,
which I think heikki/jst removed from the build. I know I never got
around to adding Includes and Dashmatch tokens to that one...)
For the code in nsCSSStyleSheet.cpp, I'd like to see a switch to the
new string code, but it doesn't have case-insensitive comparison yet
so I guess it's OK as is. You might want to short-circuit out of the
substring (Right/Left) |if (attr->mValue.Length() < value.Length())|.
I just filed bug 70740 requesting such comparison tools. The new
string code would have the (perhaps significant) performance advantage
that you don't need to copy the substring in order to compare it.
The other (very?) nasty issue here is that I don't think aCaseSensitive
(which I think basically means aIsHTML) is a correct indication of
whether we want the attribute *value* comparison to be case insensitive.
(It is accurate for the attribute *name* comparison, I think.) There are
many HTML attributes where I think a case-insensitive comparison is
inappropriate. (Shouldn't it be appropriate only for attributes with
enumerated values, or something like that?) This issue already exists in
the current code -- you're just adding more code that raises it so I
thought I'd bring it up (again?).
r=dbaron on this (although are we supposed to wait for CR before checking
it in?), although perhaps we should address the third issue at some
point, and you might want to consider the slight performance optimization
that I mentioned in the second.
Keywords: review
Comment 5•24 years ago
|
||
I agree with David that we need to understand better when an attribute *value*
is case sensitive and when it is not. For example,
<a href="http://www.foo.com/First"> is not the same as
<a href="http://www.foo.com/first"> (at least not if the server is Unix!). The
spec, as you know, begs this question off to the document language, so we need
to understand how HTML and XML specify this - at least I need to understand it
better!
I think David's proposed optimization is good too: in the patch if
attr->mValue.Length() is LESS than value.Length() then don't bother with
generating the partValue copy since none of the substring matches can succeed.
I'm very interested in the findings regarding the case sensitivity issue, in
fact I think that is critical to approval for this bug.
Comment 6•24 years ago
|
||
I forgot the correct answer to the case sensitivity issue -- it depends on
those little [CI] and [CS] markers scattered throughout the HTML spec. I
think this is an bug we've had for years, and solving it isn't trivial (we
would need to implement a table to store that information and access it
through the attributes or some other way), so I don't think we should hold
up this patch for it, although a solution in the near future would be nice.
(I think there may already be another bug on it, although I'm not sure.)
Comment 7•24 years ago
|
||
OK - I buy that. And the patch is consistent with how we were already treating
the other attribute value matching anyway.
sr=attinsi - and I see no reason to wait for CR on the selectors module to get
these in, personally. The chances of this changing are extremely close to 0.
Assignee | ||
Comment 8•24 years ago
|
||
checked in :-)
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 9•24 years ago
|
||
i vaguely remember using these recently and being happy to see them work fine,
so that classifies this bug as VERIFIED in my book. :-)
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•