Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation error for mandatory_anyof constraint when using data-amp-bind-src on amp-list #24661

Closed
westonruter opened this issue Sep 20, 2019 · 2 comments · Fixed by #24793
Closed

Comments

@westonruter
Copy link
Member

On blog.amp.dev we noticed a validation error starting to happen after ampproject/amp.dev#2934:

The tag 'amp-list' is missing a mandatory attribute - pick at least one of ['src','[src]'].

The markup being introduced included:

<amp-list data-amp-bind-src="query == undefined ? null : '/search/do?q=' + encodeURIComponent(query) + '&amp;locale=en'" binding="no" items="." height="10" layout="fixed-height" data-amp-bind-is-layout-container="query.length" load-more="auto" load-more-bookmark="nextUrl" reset-on-refresh="" single-item="">
	<template type="amp-mustache">...</template>
</amp-list>

As of ampproject/amp-wp#2974, the AMP WordPress plugin normalizes the bracketed amp-bind attribute syntax to data-amp-bind-*, so the original [src] is being converted here to data-amp-bind-src. The AMP validator then is flagging this as an error because of this mandatory_anyof constraint:

mandatory_anyof: "['src','[src]']"

A quick fix here would be to just add data-amp-bind-src to the list:

- mandatory_anyof: "['src','[src]']"
+ mandatory_anyof: "['src','[src]','data-amp-bind-src']"

However, this seems like a fix for a more fundamental issue where the validator needs to be more aware of the alternate syntax (#11115).

@Gregable
Copy link
Member

I actually think the proposed fix is fine. The validator isn't really aware of the current [foo] syntax in any special case way, these are just allowed. All data- attrs are also allowed.

Is there any other example of an issue caused by the validator being not aware of the bind syntax?

@westonruter
Copy link
Member Author

I believe that the mandatory_anyof constraint in amp-list is the only instance. I've opened #24793 to implement that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants