Per http://lists.w3.org/Archives/Public/www-style/2008Feb/0333.html we should treat attribute substring selectors with "" as the substring as parser errors. This change needs to be made in CSSParserImpl::ParseAttributeSelector in layout/style/nsCSSParser.cpp.
Created attachment 306470 [details]
Created attachment 306473 [details]
Created attachment 306476 [details] [diff] [review]
The message doesn't say anything about =~. Is that changing too?
Could you add a mochitest in layout/style/test to test for this?
=~ suffers from the same ambiguity, so I'd assume so. Peter, is that correct?
Can't I just add a reftest?
Well, I don't see how =$ and =^ suffered from any ambiguity. But I'm fine with the change, although I'd prefer not to change anything if the WG hasn't agreed to and announced it.
You could add a reftest, but mochitests are a good bit faster, especially once we start testing a lot of different selectors (which I think we should eventually).
Yes, ~= has the same problem and is changing as well.
The ambiguity is what should [foo^=""] select? All elements with a foo attribute, only foo="", or no foo attribute? ie: What strings "begin" with an empty string, all of them, none of them, or only empty strings?
The WG has decided to treat these as invalid selectors in CSS3 since they have no well defined behavior and have no purpose anyhow. The change will be updated in the Selecots draft shortly.
For mochitests, you could use the framework I put in the patch in bug 420814.
Daniel: a number of your tests at the above URL will be broken by this change. (I discovered this when investigating the solitary red block in eighth row for [attr*=""].) I also note you don't have a test for |="", although what's supposed to happen there is unclear from the URL in comment 0.
I'll update the tests, thanks Jeff.
It ([attr|=""]) is clear in http://lists.w3.org/Archives/Public/www-style/2008Mar/0007.html
Comment on attachment 306476 [details] [diff] [review]
Could you fix the |= per comments above, and add tests?
For tests, you can steal test_parseable and test_balanced_unparseable from attachment 308231 [details] [diff] [review] (this will probably land first). It might not be a bad idea to run a few test_selector_in_html tests too, though.
Created attachment 308577 [details] [diff] [review]
patch with tests
Added tests. Don't know what you mean by "fix [attr|='']", though; it's working fine afaict. Let me know if there's some other tests you want me to add here.
BTW, why is the function named test_balanced_unparseable instead of just test_unparseable?
Comment on attachment 308577 [details] [diff] [review]
patch with tests
r+sr=dbaron, given that you've actually run the mochitests you're adding and they all pass.
It would be good if the WG made some public announcement about ~=, though. That actually requires errata to CSS 2.1 as well. (And, actually, that makes me a bit hesitant about the whole thing given how long that selector has been around -- and that the current behavior is probably interoperable. In other words, I'm not particularly happy with this resolution since I don't see any good reason for deviating from the mostly-already-interoperable implementations, but I can live with it.)
Though I think I'd prefer that this not land until CSS 2.1 errata are published (and given a chance for public feedback).
Now apparently we should accept the empty string but make it match nothing.
Created attachment 324111 [details] [diff] [review]
(I hope fantasai doesn't mind)
I'll run this through tests shortly and attach my own soon, this makes the last of Glazman's CSS3 selectors tests pass.
I'm really not sure what the case is for changing interoperably-implemented behavior here.
(In reply to comment #18)
> I'm really not sure what the case is for changing interoperably-implemented
> behavior here.
What do you mean?
Are the other browsers, which all implement $="" and ^="" the same way we do, planning to change to match this as well? Are they waiting until the spec is published or in CR again or doing it now?
Frankly, I'm hesitant to make the change because I wouldn't be surprised if the spec changes again when the other implementers notice and complain.
(In reply to comment #20)
> Are the other browsers, which all implement $="" and ^="" the same way we do,
> planning to change to match this as well? Are they waiting until the spec is
> published or in CR again or doing it now?
> Frankly, I'm hesitant to make the change because I wouldn't be surprised if the
> spec changes again when the other implementers notice and complain.
Have there been any complaints about this decision in the W3 list? This resolution was made 2 months ago according to the date so there should have been plenty of time for complaints to arise.
If not, either they didn't take any notice and are just following us in the hopes of compatibility, or they support that but still follow us in the hopes of compatibility.
(In reply to comment #21)
> (In reply to comment #20)
> If not, either they didn't take any notice and are just following us in the
> hopes of compatibility, or they support that but still follow us in the hopes
> of compatibility.
No idea what you mean by following us. That said, I don't particularly care.
How about some tests with the patch? (See, for example, the tests in the previous patch?)
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #20)
> > If not, either they didn't take any notice and are just following us in the
> > hopes of compatibility, or they support that but still follow us in the hopes
> > of compatibility.
> No idea what you mean by following us. That said, I don't particularly care.
I mean that in the same way they (IIRC) stopped enforcing WRONG_DOCUMENT_ERR due to web developers coding by Gecko's standards rather than web standards. Gecko provides a huge leverage in the standards community due to its popularity.
I will be attaching tests shortly.
No, this is in the category of something that nobody ever tried or cared about until somebody wrote a selectors test suite that tested it, not something that Web pages would actually depend on. (It's also a pretty new feature compared to that.)
Created attachment 324118 [details] [diff] [review]
Created attachment 324119 [details] [diff] [review]
It makes more sense to do the checking this way.
Comment on attachment 324119 [details] [diff] [review]
Comment on attachment 324118 [details] [diff] [review]
r=dbaron, if all four of these are also preceded by a test_parseable for the same selector.
We should also update and land fantasai's tests.
Created attachment 324124 [details] [diff] [review]
I'm preparing these for landing in my tree, but I might not actually land until later this evening.
And some of fantasai's tests were still omitted in your "more tests" patch, but I think I've added them all back now.
Thanks for the patches.
Looks good with the following builds. So marking it verified.
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a1pre) Gecko/2008071002 Minefield/3.1a1pre ID:2008071002
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008070903 Minefield/3.1a1pre ID:2008070903