Bug 16751 - Failing to parse "html*.test" correctly as CSS selector (Acid3 bug)
Summary: Failing to parse "html*.test" correctly as CSS selector (Acid3 bug)
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL: http://www.hixie.ch/tests/evil/acid/003/
Keywords: HasReduction
Depends on: 15508
Blocks: Acid3
  Show dependency treegraph
Reported: 2008-01-05 14:22 PST by Eric Seidel (no email)
Modified: 2008-02-04 14:16 PST (History)
1 user (show)

See Also:

Minimal test case (319 bytes, text/html)
2008-01-27 15:32 PST, Robert Blaut
no flags Details
Patch that fixes the bug (5.52 KB, patch)
2008-02-04 12:00 PST, Dave Hyatt
no flags Details | Formatted Diff | Diff
Shrink the patch a bit. (5.29 KB, patch)
2008-02-04 13:15 PST, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-01-05 14:22:03 PST
Failing to parse "html*.test" correctly as CSS selector (Acid3 bug)

Test 44: FAIL (expected 0, got 1 - misparsed selectors)

    function () {
      // test 44: selectors without spaces before a "*"
      selectorTest(function (doc, add, expect) {
        doc.body.className = "test";
        var p = doc.createElement('p');
        p.className = "test";
        expect(doc.body, 0, "misparsed selectors");
        expect(p, 0, "really misparsed selectors");
      return 3;
Comment 1 Robert Blaut 2008-01-27 15:32:22 PST
Created attachment 18727 [details]
Minimal test case
Comment 2 Dave Hyatt 2008-02-04 12:00:34 PST
Created attachment 18913 [details]
Patch that fixes the bug

Will land test case from bug as layout test.
Comment 3 Dave Hyatt 2008-02-04 13:15:18 PST
Created attachment 18917 [details]
Shrink the patch a bit.
Comment 4 Darin Adler 2008-02-04 13:38:37 PST
Comment on attachment 18917 [details]
Shrink the patch a bit.

This looks like it relies on how bison handles conflicts. Anywhere we say "selector" it could expand to "selector_with_trailing_whitespace" so it's all about how it reduces.

Do we have sufficient test coverage for CSS selector parsing? I'd like to see a lot of tests in this area.

r=me, more based on the theory that this is thoroughly tested rather than on my certainty that I can validate this from proofreading the code.

I'd also like to see this grammar fixed to remove all the conflicts; that would make it easier to understand the behavior by looking at the code, although it would make the grammar more complex.
Comment 5 Dave Hyatt 2008-02-04 14:10:36 PST
I don't believe it does rely on how Bison handles conflicts.  The patch forces whitespace following a selector to resolve into "selector_with_trailing_whitespace."  There is no other reduce option for whitespace following a selector.

Comment 6 Dave Hyatt 2008-02-04 14:16:56 PST
Fixed in r29976-8.