Sanitizer: Request to convert <img> to <amp-img> correctly

[kienstra]

As @westonruter mentioned in the 'ampconf' theme, the sanitizer doesn't look to be converting <img> tags to <amp-img> correctly:

• width and height attributes are unexpectedly different
• Changes in the srcset

Here's @westonruter's temporary workaround for reference.

[kienstra]

Will Work On After #864

Hi @westonruter,
If it's alright, I'll work on this after the video issue in #864.

[kienstra]

Request For Another Look

Hi Koop or @westonruter,
Could you please let me know if you still see this issue? I can't reproduce the bug described in this issue, and mentioned in Theme Issue 48. Here are my results from looking at a single.php page. Maybe something fixed it in the meantime.

Without the AMP plugin active,

the_post_thumbnail( 'ampconf-1040x400' );

produced:

<img width="1040" height="400" src="http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-1040x400.jpg" class="attachment-ampconf-1040x400 size-ampconf-1040x400 wp-post-image" alt="" srcset="http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-1040x400.jpg 1040w, http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-768x295.jpg 768w" sizes="(max-width: 1040px) 100vw, 1040px">

With the plugin active, that function seems to produce the same width, height, and srcset:

<amp-img width="1040" height="400" src="http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-1040x400.jpg" class="attachment-ampconf-1040x400 size-ampconf-1040x400 wp-post-image amp-wp-enforced-sizes i-amphtml-element i-amphtml-layout-responsive i-amphtml-layout-size-defined i-amphtml-layout" alt="" srcset="http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-1040x400.jpg 1040w, http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-768x295.jpg 768w" sizes="(min-width: 694px) 694px, 100vw" style="width: 694px;"><i-amphtml-sizer style="display: block; padding-top: 38.4615%;"></i-amphtml-sizer><img decoding="async" alt="" class="i-amphtml-fill-content i-amphtml-replaced-content" src="http://local.envi/wp-content/uploads/2018/01/American_bison_k5680-1-1040x400.jpg"></amp-img>

There is a difference in the sizes values. Is this an issue?

Without plugin:

sizes="(max-width: 1040px) 100vw, 1040px"

With plugin:

sizes="(min-width: 694px) 694px, 100vw"

There were similar results when I deleted the image, changed to theme to Twenty Seventeen, uploaded the image, set it as the featured image, and changed back to the ampconf theme. There wasn't much difference between the output with the plugin activated or deactivated.

Theme: develop branch, with the workaround commit reverted
Plugin: develop branch

Thanks 😄

[westonruter]

I think the issue is that the layout is being inferred incorrectly in some cases: https://www.ampproject.org/docs/guides/responsive/control_layout#what-if-the-layout-attribute-isn%E2%80%99t-specified?

the solution may just be that we need to explicitly set the layout when it matters.

[westonruter]

@kienstra correct me if I'm wrong, but I believe this is resolved as of merging #937.

[kienstra]

Hi @westonruter, yes, this issue is resolved now that #937 is merged.