Amphtml: amp-iframe: Add support for message protocols of popular libraries that do resizable iframes

Created on 5 Jun 2019  ·  19Comments  ·  Source: ampproject/amphtml

Many sites use libraries like Pym.js to implement resizable iframes. In AMP, amp-iframe has its own message protocol for implementing resizable iframes (https://github.com/ampproject/amphtml/issues/728).

Currently, in order to take existing legacy content and put it into AMP pages, they have to be updated to use the the AMP-specific message protocol. It would be great if amp-iframe could be made aware of the protocols of popular resizable iframe libraries (like Pym.js), as this would facilitate migration to AMP.

This was proposed by @thomaswilburn in https://github.com/nprapps/pym.js/issues/182#issuecomment-499266791:

Alternatively, if the AMP project actually cares about working with the community, perhaps they should implement compatibility with legacy Pym messages. There are a lot of static graphics generated with tools like our Dailygraphics Rig that are not going to be upgraded to a new Pym or Sidechain, because it's not reasonable to dig all of them up and republish them. Having AMP support the news industry's de-facto embedding standard would be a real sign of goodwill.

For more historical context: https://github.com/ampproject/amphtml/issues/495

When Possible Feature Request runtime

Most helpful comment

Very excited for #24917 @dvoytenko @westonruter. This could fix hundreds of pages on Axios, thousands if you include the previously mentioned news sites 👏🎉📈

All 19 comments

/cc @aghassemi

/cc @cramforce @pbakaus @dvoytenko @rudygalfi

We've made changes like this in the past.

How does their protocol look like? I couldn't quickly find docs.

Messages are serialized into a string, prefixed by pym with arguments joined by the delimiter xPYMx:

https://github.com/nprapps/pym.js/blob/57feb680ac3ff7aeef080e5efe0ddbe665530eac/src/pym.js#L85-L102

So a message to update the height of the FOO iframe to 300 pixels, I believe would look like:


IMHO, there's no problem to support 1-2 very common resize message formats. But I'd stay away from using the libraries themselves - they often have their own legacy overhead.

Agreed. Support the protocols but don't incorporate the libraries.

Weird flex but OK :)

I had never heard of Pym. How common is it?

It's probably the most common solution used by newsroom interactive teams today. It's used at NPR, almost all of our members stations, as well as (I think) the Texas Tribune, St. Louis Post-Dispatch, INN-supported newsrooms, and many others.

The message format isn't great, but we don't have the ability to change it now, it was implemented long before I joined the team. We have introduced a new Pym- and AMP-compatible solution called Sidechain, but it isn't in production use yet. And due to the way that we publish graphics to static file hosting, it's not feasible to go back and upgrade them all.

For context, our CDN distribution for pym.nprapps.org, which is used by us and by other people who embed with this script, sees about 2.5-4M requests per day. That's not the complete story, as many users probably have their own customized version or they bundle it in from the NPM package, but it's not nothing either.

Pym.js/AMP support has also been requested by a few publications in the Newspack world, including Reveal, The Rivard Report, The Chicago Reporter, and The Hechinger Report.

INN maintains a plugin that wraps a Pym.js embed in a shortcode or block: https://github.com/INN/pym-shortcode https://wordpress.org/plugins/pym-shortcode/

If there's something we can do to make that work better with the WordPress AMP plugin, please chime in on the Pym.js Embeds plugin's AMP support issue: https://github.com/INN/pym-shortcode/issues/59

(and if there's stuff that needs to be done to make the plugin compatible with Newspack, we'd like to know so we can get that implemented, too.)

Ideally, sender would be updated to send both this and other formats.

if that is infeasible, I think it is fine to add support for commonly used message formats on the receiver end.

PYM support is something we considered some time ago. Just didn't get to it. One point of disconnect: I don't believe PYM supports the "rejection" signal when we deny resize. That was a bit of a concern. Not an issue for user-gesture-based resizing, but definitely could be an issue for onload.

@dvoytenko would this be a blocker? Or can we move to add protocol support to make libraries like Pym.js AMP compatible?

@westonruter No, I don't think it'd be a blocker. We could always follow the same protocol and send "rejection" message as well, in case anyone can react to it.

@dvoytenko Finally opened a PR: #24917.

IMHO, there's no problem to support 1-2 very common resize message formats. But I'd stay away from using the libraries themselves - they often have their own legacy overhead.

I just listened to @calebcordry's talk just now about an “Sustaining an ever-growing AMP component library” at the AMP Contributors Summit; it got me thinking about whether there should be a separate WordPress embed component (#18378, amp-wordpress-embed) as opposed to extending amp-iframe. In the PR for augmenting amp-iframe to support Pym.js messages (#24917), the logic is essentially identical to the core of the amp-wordpress-embed being proposed in #24952.

Instead of listening for message events with data containing Pym.js serialized string, the amp-wordpress-embed is listening for message events containing data that has an object with keys for message and value, where message can either be height or link. The latter one is the unique one for WordPress, as instead of requiring theallow-top-navigation-by-user-activation sandbox keyword, WordPress sends a link message with the desired URL and then the parent window only follows the URL if it with the same origin as the window being embedded.

All that to say, the logic needed for WordPress embeds is similar to Pym.js, though it does have the additional facility for navigation. Does this make WordPress embeds warrant having a separate extension?

@westonruter I think we should view pym support and the amp-wordpress-embed separately for now, b/c pym is used in non-wordpress embeds as well, right? Otherwise, imho, you should take for amp-wordpress-embed to design review.

Very excited for #24917 @dvoytenko @westonruter. This could fix hundreds of pages on Axios, thousands if you include the previously mentioned news sites 👏🎉📈

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jasti picture jasti  ·  50Comments

zhouyx picture zhouyx  ·  60Comments

choumx picture choumx  ·  42Comments

retornam picture retornam  ·  52Comments

sebastianbenz picture sebastianbenz  ·  48Comments