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
: WebKit
CSS
: 528+ (Nightly build)
: Macintosh Mac OS X 10.4
: P2 Normal
Assigned To:
: http://www.hixie.ch/tests/evil/acid/003/
: HasReduction
: 15508
: 17064
  Show dependency treegraph
 
Reported: 2008-01-05 14:22 PST by
Modified: 2008-02-04 14:16 PST (History)


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 Review Patch | Details | Formatted Diff | Diff
Shrink the patch a bit. (5.29 KB, patch)
2008-02-04 13:15 PST, Dave Hyatt
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 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 From 2008-01-27 15:32:22 PST -------
Created an attachment (id=18727) [details]
Minimal test case
------- Comment #2 From 2008-02-04 12:00:34 PST -------
Created an attachment (id=18913) [details]
Patch that fixes the bug

Will land test case from bug as layout test.
------- Comment #3 From 2008-02-04 13:15:18 PST -------
Created an attachment (id=18917) [details]
Shrink the patch a bit.
------- Comment #4 From 2008-02-04 13:38:37 PST -------
(From update of attachment 18917 [details])
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 From 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 From 2008-02-04 14:16:56 PST -------
Fixed in r29976-8.