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

See Also:


Attachments
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 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";
        doc.body.appendChild(p);
        add("html*.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.