Microdata item with itemprop attribute should not include the item itself in the HTMLPropertiesCollection

• Component: DOM
• Status: RESOLVED FIXED

[Arko Saha]

Currently we add microdata item in its property collection if item has an itemprop attribute specified.

Test:
var testEl = makeEl('div',{itemscope:'itemscope'},'<div itemscope itemprop="foo"><div itemprop="bar"></div></div>');
var item = testEl.firstChild;
assert_equals( item.properties.length, 1, 'properties length')

Expected: PASS (properties length must be 1)
Actual: FAIL- properties length expected 1 but got 2 (foo and bar)

item (inner div) has itemprop attribute specified with value 'foo'. It has only one property 'bar'.
So, item.propeties.length must return 1.

[Arko Saha]

Created attachment 157728 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 157728 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157728&action=review

> Source/WebCore/dom/PropertyNodeList.cpp:86
> -        if (testElement == m_itemRefElementsCache[i] || elementIsPropertyOfRefElement(testElement, m_itemRefElementsCache[i])) {
> +        Node* refElement = m_itemRefElementsCache[i];
> +        if (testElement ==  refElement || elementIsPropertyOfRefElement(testElement, refElement)) {

Why are we making this change?

> Source/WebCore/html/HTMLPropertiesCollection.cpp:96
> +    Node* ownerNode = this->base();

Can we call ownerNode() instead here so that there is no confusion about base vs. ownerNode?

> Source/WebCore/html/HTMLPropertiesCollection.cpp:99
> +    for (; current; current = nextNodeWithProperty(base, current, ownerNode)) {

We should probably rename base to rootNode here. It's too confusing.
(make sure to update terms in nextNodeWithProperty.

[Arko Saha]

Created attachment 157838 [details]
Updated patch

Incorporated review comments.

[WebKit Review Bot]

Comment on attachment 157838 [details]
Updated patch

Clearing flags on attachment: 157838

Committed r125348: <http://trac.webkit.org/changeset/125348>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.