Bug 93456 - [Microdata] HTMLPropertiesCollection does not contain all properties when item is not attached to the DOM tree
Summary: [Microdata] HTMLPropertiesCollection does not contain all properties when ite...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Arko Saha
URL:
Keywords:
Depends on:
Blocks: 92986
  Show dependency treegraph
 
Reported: 2012-08-08 02:23 PDT by Arko Saha
Modified: 2012-08-09 00:28 PDT (History)
2 users (show)

See Also:


Attachments
Patch (20.34 KB, patch)
2012-08-08 03:00 PDT, Arko Saha
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arko Saha 2012-08-08 02:23:54 PDT
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
Comment 1 Arko Saha 2012-08-08 03:00:50 PDT
Created attachment 157165 [details]
Patch
Comment 2 Ryosuke Niwa 2012-08-08 11:12:00 PDT
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?
Comment 3 Arko Saha 2012-08-08 23:21:07 PDT
(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?
Comment 4 Ryosuke Niwa 2012-08-08 23:24:38 PDT
(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.
Comment 5 Arko Saha 2012-08-08 23:28:38 PDT
(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.
Comment 6 WebKit Review Bot 2012-08-09 00:28:30 PDT
Comment on attachment 157165 [details]
Patch

Clearing flags on attachment: 157165

Committed r125157: <http://trac.webkit.org/changeset/125157>
Comment 7 WebKit Review Bot 2012-08-09 00:28:34 PDT
All reviewed patches have been landed.  Closing bug.