amp-wp
amp-wp copied to clipboard
Skip rendering child amp-img tags of amp-mega-menu
Bug Description
A user reported a that he encounter a validation error when he tries to add amp-img under amp-mega-menu component. the validation error is regarding sizer being descendent of amp-mega-menu which is not allowed.
I am able to reprorudcue the issue at my end using the following HTML snippet
<amp-mega-menu height="30" layout="fixed-height">
<nav>
<ul>
<li>
<span role="button">Image</span>
<div role="dialog">
<amp-img
src="https://amp-support.rt.gw/wp-content/uploads/2008/06/dsc09114-1-1024x768.jpg"
width="300"
height="200"></amp-img>
</div>
</li>
<li>
<span role="button">List</span>
<div role="dialog">
<ol>
<li>item 1</li>
<li>item 2</li>
<li>item 3</li>
</ol>
</div>
</li>
<li>
<a href="https://amp.dev/">Link</a>
</li>
</ul>
</nav>
</amp-mega-menu>
Users Site: https://www.nmore.com/ My testing site. : https://amp-support.rt.gw/amp-mega-menu-with-image/?amp=1
Expected Behaviour
It should not produce validation error.
Screenshots
No response
PHP Version
7.4
Plugin Version
2.2.2
AMP plugin template mode
Transitional
WordPress Version
5.9.4
Site Health
No response
Gutenberg Version
No response
OS(s) Affected
No response
Browser(s) Affected
No response
Device(s) Affected
No response
Acceptance Criteria
No response
Implementation Brief
No response
QA Testing Instructions
No response
Demo
No response
Changelog Entry
No response
Seems like this may be a bug with the AMP validator. Namely, i-amphtml-sizer
is not included among this list: https://github.com/ampproject/amphtml/blob/cb67fc9827971df8d0ca0ad0b5b123496323b03a/extensions/amp-mega-menu/validator-amp-mega-menu.protoascii#L153-L209
Now that native img
elements are valid AMP, I think the best short-term fix would be to just opt for native img
via:
add_filter( 'amp_native_img_used', '__return_true' );
Oh, but img
isn't in that list either!
I've opened https://github.com/ampproject/amphtml/pull/38028 to address this.
The resolution for this issue is to update the plugin's validator spec once the amphtml PR is merged.
We need to wait for https://github.com/ampproject/amphtml/pull/38028 to be merged.
The issue is not reproducible with the current version of the plugin. It appears to be functioning correctly.
Seems like it's not working as expected because img
and i-amphtml-sizer
are not added in amp-mega-menu-allowed-descendants
even though they exist in the spec now.
https://github.com/ampproject/amp-wp/blob/789e09967598344eddedab128ef0066f5e31c516/includes/sanitizers/class-amp-allowed-tags-generated.php#L22-L76
The reason why they are not included in the allowed descendants is this spec generator code https://github.com/ampproject/amp-wp/blob/789e09967598344eddedab128ef0066f5e31c516/bin/amphtml-update.py#L420-L432
QA Passed ✅
Cross-verified the issue and it is working fine. The Image tag error is not visible now.
Before fix | After fix |
---|---|