Bug 70501 - [Microdata] itemtype attribute should be space-separated list to allow multiple types
Summary: [Microdata] itemtype attribute should be space-separated list to allow multip...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 68609
  Show dependency treegraph
 
Reported: 2011-10-20 08:01 PDT by Arko Saha
Modified: 2011-10-25 09:53 PDT (History)
8 users (show)

See Also:


Attachments
Patch (17.68 KB, patch)
2011-10-20 08:14 PDT, Arko Saha
no flags Details | Formatted Diff | Diff
Updated patch (16.23 KB, patch)
2011-10-25 01:20 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 2011-10-20 08:01:49 PDT
Incorporating W3C the spec changes:

1. Microdata itemtype attribute is now a space-separated list of types rather than a single type, so that it can allow multiple types if they share the same vocabulary.
This is added in the spec in the revision number : http://html5.org/r/6668.
Spec: http://www.whatwg.org/specs/web-apps/current-work/#attr-itemtype

2. getItems() should return all the elements in the document that are each top-level microdata items whose types include all the types specified in the method's argument.
This is added in the spec in the revision number : http://html5.org/r/6680.
Spec: http://www.whatwg.org/specs/web-apps/current-work/#dom-document-getitems
Comment 1 Arko Saha 2011-10-20 08:14:36 PDT
Created attachment 111776 [details]
Patch
Comment 2 Ryosuke Niwa 2011-10-24 14:51:35 PDT
Comment on attachment 111776 [details]
Patch

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

> LayoutTests/fast/dom/MicroData/006-expected.txt:5
>  PASS document.getItems(' http://example.com/foo http://example.com/bar ').length == 2 is true
> -PASS document.getItems(' http://example.com/foo data:text/plain ').length == 2 is true
> -PASS document.getItems(' http://example.com/foo data:text/plain http://example.com/foo').length == 2 is true
> +PASS document.getItems('  http://example.com/foo  http://example.com/bar  http://example.com/foo').length == 2 is true

Why the change?

> LayoutTests/fast/dom/MicroData/006.html:11
> -<div itemscope itemtype="http://example.com/foo"></div>
> -<div itemscope itemtype="http://example.com/bar">
> -<div itemscope itemtype="data:text/plain"></div>
> +<div itemscope itemtype="http://example.com/foo http://example.com/bar"></div>
> +<div itemscope itemtype="http://example.com/bar http://example.com/foo">

Ditto.

> LayoutTests/fast/dom/MicroData/006.html:16
> -shouldBeTrue("document.getItems(' http://example.com/foo  data:text/plain  ').length == 2");
> -shouldBeTrue("document.getItems('  http://example.com/foo  data:text/plain  http://example.com/foo').length == 2");
> +shouldBeTrue("document.getItems('  http://example.com/foo  http://example.com/bar  http://example.com/foo').length == 2");

Ditto.

> LayoutTests/fast/dom/MicroData/itemtype-add-remove-tokens.html:35
> +debug("<br>itemType.remove must not make any changes if a non-existing token is removed.");

"non-existing token is removed" sounds odd. How about "remove is called for a token that doesn't exist"?
Comment 3 Arko Saha 2011-10-25 01:13:27 PDT
(In reply to comment #2)
> (From update of attachment 111776 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111776&action=review
> 

> > LayoutTests/fast/dom/MicroData/006.html:11
> > -<div itemscope itemtype="http://example.com/foo"></div>
> > -<div itemscope itemtype="http://example.com/bar">
> > -<div itemscope itemtype="data:text/plain"></div>
> > +<div itemscope itemtype="http://example.com/foo http://example.com/bar"></div>
> > +<div itemscope itemtype="http://example.com/bar http://example.com/foo">
> 
> Why the change?

Accroding to the latest spec getItems() should return all the elements that are each top-level microdata items whose types include "all the types specified" in the method's argument.
Earlier the spec specified that getItems() should return all the elements that are each top-level microdata items whose types include "any one of the types specified" in the method's argument.

So if we do not modify the below test-case,

-<div itemscope itemtype="http://example.com/foo"></div>
-<div itemscope itemtype="http://example.com/bar">
+<div itemscope itemtype="http://example.com/foo http://example.com/bar"></div>
+<div itemscope itemtype="http://example.com/bar http://example.com/foo">

document.getItems(' http://example.com/foo http://example.com/bar ').length == 2 will always be false. Hence I made this changes.
And I have modified this test-case more perfect in my new patch.

> 
> > LayoutTests/fast/dom/MicroData/itemtype-add-remove-tokens.html:35
> > +debug("<br>itemType.remove must not make any changes if a non-existing token is removed.");
> 
> "non-existing token is removed" sounds odd. How about "remove is called for a token that doesn't exist"?
Done!
Comment 4 Arko Saha 2011-10-25 01:20:17 PDT
Created attachment 112305 [details]
Updated patch
Comment 5 WebKit Review Bot 2011-10-25 09:53:03 PDT
Comment on attachment 112305 [details]
Updated patch

Clearing flags on attachment: 112305

Committed r98353: <http://trac.webkit.org/changeset/98353>
Comment 6 WebKit Review Bot 2011-10-25 09:53:08 PDT
All reviewed patches have been landed.  Closing bug.