Bug 16751 – Failing to parse "html*.test" correctly as CSS selector (Acid3 bug)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Eric Seidel (no email)]

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;
    },

[Robert Blaut]

Created attachment 18727 [details]
Minimal test case

[Dave Hyatt]

Created attachment 18913 [details]
Patch that fixes the bug

Will land test case from bug as layout test.

[Dave Hyatt]

Created attachment 18917 [details]
Shrink the patch a bit.

[Darin Adler]

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.

[Dave Hyatt]

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.

[Dave Hyatt]

Fixed in r29976-8.