Bug 11387 - CSS3: :empty selector is applied to all elements (Acid3 bug)
Summary: CSS3: :empty selector is applied to all elements (Acid3 bug)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL: http://www.css3.info/selectors-test/t...
Keywords: HasReduction
Depends on:
Blocks: Acid3 11390 17125
  Show dependency treegraph
 
Reported: 2006-10-23 03:12 PDT by Niels Leenheer (HTML5test)
Modified: 2008-02-01 11:23 PST (History)
4 users (show)

See Also:


Attachments
test case (517 bytes, text/html)
2008-01-30 13:21 PST, Robert Blaut
no flags Details
Patch that implements dynamic :empty (68.94 KB, patch)
2008-01-31 14:34 PST, Dave Hyatt
mitz: review+
Details | Formatted Diff | Diff
Revised patch with refinements from Acid3 testing. (69.39 KB, patch)
2008-01-31 18:31 PST, Dave Hyatt
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Niels Leenheer (HTML5test) 2006-10-23 03:12:59 PDT
Current behavoir:
The :empty selector matches every element.

Reason:
The reason for this is that every time a new element is added to the DOM, it is checked to see if it matches one of the CSS selectors. Because the DOM is build one element at a time, every element is at least for a brief moment empty. 

Proper behavoir:
The :empty selector should only match elements that are *really* empty. Once an element is no longer empty it should no longer match this selector.
Comment 1 Eric Seidel (no email) 2008-01-01 22:13:41 PST
I think Test 37 in Acid3 is hitting this bug.
Comment 2 Robert Blaut 2008-01-30 13:21:00 PST
Created attachment 18798 [details]
test case
Comment 3 Robert Blaut 2008-01-30 13:22:44 PST
Currently it's a test 38 in Acid3:

    function () {
      // test 38: :empty
      selectorTest(function (doc, add, expect) {
        var empty = add(":empty");
        var p = doc.createElement('p');
        doc.body.appendChild(p);
        expect(p, empty, "empty element didn't match :empty");
        var span = doc.createElement('span');
        p.appendChild(span);
        expect(p, 0, "removing all children didn't make the element match :empty");
        p.removeChild(span);
        p.appendChild(doc.createComment("c"));
        p.appendChild(doc.createTextNode("a"));
        expect(p, 0, "element with no child element nodes didn't match :empty");
      });
      return 3;
    },
Comment 4 Ian 'Hixie' Hickson 2008-01-30 16:25:26 PST
Wow, that test had errors. Fixed. Doesn't affect this bug, though.
Comment 5 Dave Hyatt 2008-01-31 14:34:03 PST
Created attachment 18831 [details]
Patch that implements dynamic :empty
Comment 6 Oliver Hunt 2008-01-31 14:55:37 PST
Comment on attachment 18831 [details]
Patch that implements dynamic :empty

The formatting change in CSSStyleSelector::locateSharedStyleseems unnecessary.

This looks good to me, but given mitz spotted the issue with comment nodes breaking the hasChildNodes() == style->emptyState() check and i didn't, i figure i'll leave this to him.
Comment 7 mitz 2008-01-31 15:29:58 PST
Comment on attachment 18831 [details]
Patch that implements dynamic :empty

r=me with the condition changed to (!style->emptyState() || hasChildNodes()).
Comment 8 Dave Hyatt 2008-01-31 18:31:00 PST
Created attachment 18840 [details]
Revised patch with refinements from Acid3 testing.
Comment 9 Oliver Hunt 2008-01-31 19:06:31 PST
Comment on attachment 18840 [details]
Revised patch with refinements from Acid3 testing.

r=me
Comment 10 Dave Hyatt 2008-02-01 11:23:25 PST
Fixed in r29918.