Skip to content

Add valid attribute entries for schema.org RDFa syntax#6739

Merged
Gregable merged 5 commits intoampproject:masterfrom
scienceai:validator-rdfa
Dec 20, 2016
Merged

Add valid attribute entries for schema.org RDFa syntax#6739
Gregable merged 5 commits intoampproject:masterfrom
scienceai:validator-rdfa

Conversation

@darobin
Copy link
Copy Markdown
Contributor

@darobin darobin commented Dec 19, 2016

AMP is currently in the somewhat contradictory state of encouraging schema.org and considering one of the syntaxes used for that sort of annotation (namely RDFa) invalid. Accepting RDFa is harmless in terms of performance (browsers don't do anything with it).

This was reported in #4919.

@jridgewell
Copy link
Copy Markdown
Contributor

/to @Gregable

Copy link
Copy Markdown
Member

@Gregable Gregable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR addressing this issue!

attrs: { name: "itemtype" }
# Also for schema.org, RDFa syntax
attrs: { name: "about" }
attrs: { name: "content" }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these are fine, except content. We have some specific rules related to the content attribute for <meta> tags that we need to make sure we still are enforcing, for example <meta name=viewport content=...>.

I think the non-global attribute rules will override the global ones, but if we are adding this, it would be good to add a test which demonstrates this. Can you add a feature test showing that this fails validation:

<meta name=viewport content=invalid>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I have added a check that the meta rules are still enforced, clearly the more specific rules get picked up over generic ones. This also led me to notice that there were specific rules for link that would break with RDFa, so I have added a rule for that case. It is modelled on the itemprop one and (just like itemprop before it) it doesn't break link in other cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also, sorry for the duplicated git commit message, I did run git commit --amend on that one but clearly I failed at it.)

@Gregable
Copy link
Copy Markdown
Member

Looks good to me. I'll go ahead and merge.

Just to warn you, it'll be a few weeks before this change gets released.

@Gregable Gregable merged commit e106265 into ampproject:master Dec 20, 2016
@darobin
Copy link
Copy Markdown
Contributor Author

darobin commented Dec 20, 2016

Understood, thanks a lot! If there's any publicly-documented information about when it might ship I would be thankful, otherwise I'll just wait!

@Gregable
Copy link
Copy Markdown
Member

Unfortunately, the schedule is a bit unpredictable.

Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
* add valid attribute entries for schema.org RDFa syntax

* add tests for RDFa validation support

* check that adding RDFa did not break meta@content

* check that adding RDFa did not break meta@content

* add a rule for RDFa link
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
* add valid attribute entries for schema.org RDFa syntax

* add tests for RDFa validation support

* check that adding RDFa did not break meta@content

* check that adding RDFa did not break meta@content

* add a rule for RDFa link
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Jan 3, 2017
* add valid attribute entries for schema.org RDFa syntax

* add tests for RDFa validation support

* check that adding RDFa did not break meta@content

* check that adding RDFa did not break meta@content

* add a rule for RDFa link
@danbri
Copy link
Copy Markdown
Contributor

danbri commented Jan 10, 2017

This is great, thanks! :)

From a schema.org perspective I might mention that our use of the content attribute has leaked across from RDFa into various of the Microdata examples on schema.org even if it is not historically part of the Microdata story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants