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)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: glazou, Assigned: glazou)

Details

Attachments

(1 file)

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...
Status: NEW → ASSIGNED
proposed implementation ; tested with CSS 3 test suite ==> green :-))) r= needed
Review keyword, cc self. Looking forward to seeing CSS3 as a full rec.
Keywords: review
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
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.
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.)
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.
checked in :-)
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: