Bug 11387

Summary:

CSS3: :empty selector is applied to all elements (Acid3 bug)

Product:

WebKit

Reporter:

Niels Leenheer (HTML5test) <info>

Component:

CSS

Assignee:

Dave Hyatt <hyatt>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, ian, mrowe, webkit

Priority:

Keywords:

HasReduction

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

URL:

http://www.css3.info/selectors-test/test-empty.html#empty

Bug Depends on:

Bug Blocks:

11390, 17064, 17125

Attachments:

Description	Flags
test case	none
Patch that implements dynamic :empty	mitz: review+
Revised patch with refinements from Acid3 testing.	oliver: review+

Niels Leenheer (HTML5test)

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2008-01-01 22:13:41 PST

I think Test 37 in Acid3 is hitting this bug.

Robert Blaut

Comment 2 2008-01-30 13:21:00 PST

Created attachment 18798 [details] test case

Robert Blaut

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

Ian 'Hixie' Hickson

Comment 4 2008-01-30 16:25:26 PST

Wow, that test had errors. Fixed. Doesn't affect this bug, though.

Dave Hyatt

Comment 5 2008-01-31 14:34:03 PST

Created attachment 18831 [details] Patch that implements dynamic :empty

Oliver Hunt

Comment 6 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.

mitz

Comment 7 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()).

Dave Hyatt

Comment 8 2008-01-31 18:31:00 PST

Created attachment 18840 [details] Revised patch with refinements from Acid3 testing.

Oliver Hunt

Comment 9 2008-01-31 19:06:31 PST

Comment on attachment 18840 [details] Revised patch with refinements from Acid3 testing. r=me

Dave Hyatt

Comment 10 2008-02-01 11:23:25 PST

Fixed in r29918.

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