[Microdata] HTMLPropertiesCollection does not contain all properties when item is not attached to the DOM tree

• Component: DOM
• Status: RESOLVED FIXED

[Arko Saha]

HTMLPropertiesCollection does not find the item properties added through itemref attribute when item is not attached to the DOM tree.
We should find the root node of the item if its not attached in the document.

Test:
var parent = makeEl('div', {}, '<div itemprop="foo" id="id1"></div><div itemscope itemref="id1"></div>');
var item = parent.childNodes[1];
assert_equals(item.properties.length, 1);

Expected: item.properties.length must be 1
Actual: FAIL- item.properties.length is 0

[Arko Saha]

Created attachment 157165 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 157165 [details]
Patch

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

> Source/WebCore/dom/DynamicNodeList.cpp:45
> +        Node* root = m_ownerNode.get();
> +        while (Node* parent = root->parentNode())
> +            root = parent;

Should we do this for labels node list and radio node list also?

[Arko Saha]

(In reply to comment #2)
> (From update of attachment 157165 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157165&action=review
> 
> > Source/WebCore/dom/DynamicNodeList.cpp:45
> > +        Node* root = m_ownerNode.get();
> > +        while (Node* parent = root->parentNode())
> > +            root = parent;
> 
> Should we do this for labels node list and radio node list also?

Sure. Is is ok to file a new bug to resolve it as we need to add test cases for the same?

[Ryosuke Niwa]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 157165 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=157165&action=review
> > 
> > > Source/WebCore/dom/DynamicNodeList.cpp:45
> > > +        Node* root = m_ownerNode.get();
> > > +        while (Node* parent = root->parentNode())
> > > +            root = parent;
> > 
> > Should we do this for labels node list and radio node list also?
> 
> Sure. Is is ok to file a new bug to resolve it as we need to add test cases for the same?

Yes. In fact, that's probably preferable.

[Arko Saha]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 157165 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=157165&action=review
> > > 
> > > > Source/WebCore/dom/DynamicNodeList.cpp:45
> > > > +        Node* root = m_ownerNode.get();
> > > > +        while (Node* parent = root->parentNode())
> > > > +            root = parent;
> > > 
> > > Should we do this for labels node list and radio node list also?
> > 
> > Sure. Is is ok to file a new bug to resolve it as we need to add test cases for the same?
> 
> Yes. In fact, that's probably preferable.

Ok. Thanks for the clarification.

[WebKit Review Bot]

Comment on attachment 157165 [details]
Patch

Clearing flags on attachment: 157165

Committed r125157: <http://trac.webkit.org/changeset/125157>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.