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

amp-story and amp-iframe #39708

Closed
jaygray0919 opened this issue Dec 20, 2023 · 4 comments
Closed

amp-story and amp-iframe #39708

jaygray0919 opened this issue Dec 20, 2023 · 4 comments
Assignees

Comments

@jaygray0919
Copy link

Description

problem with amp-iframe when used with amp-story

Reproduction Steps

at some historical point, amp-iframe was valid when used in amp-story-attachment
this example works
https://afdsi.com/_google/attachment-iframe-multiple-tabs-prism-json/

but is invalid
https://search.google.com/test/amp/result?id=8eMDrRKsPPmj0fi1VVCksA

also tabs are not selectable as demonstrated in #39707

note that the amp-iframe image can be resized (which is good)

how to make the tab selector work?

also, please validate amp-iframe when used in amp-story attachment

Relevant Logs

No response

Browser(s) Affected

Chrome, Firefox, Safari

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response

@newmuis
Copy link
Contributor

newmuis commented Dec 22, 2023

I believe this has always been invalid? At least for a long time.

I believe last we checked (which was admittedly probably 4 years ago) it was not possible for the runtime to tell that an iframe had successfully loaded cross-browser. Specifically, if the site inside the frame does not allow itself to be framed, there was no way for us to be able to tell the difference.

Additionally, allowing iframes to be interactive (i.e. receive pointer events) breaks the swipe down to close mechanism, since the inner document gets the pointer events and the story does not know about them.

@jaygray0919
Copy link
Author

WRT

it was not possible for the runtime to tell that an iframe had successfully loaded cross-browser

We are loading content from sites we own/control - not 3rd party sites. As illustrated in example, amp-iframe works.
So we are hoping that the validator can be updated to accept the amp-iframe if it is part of story-attachment (descendent of that section).

Not quite following this:

allowing iframes to be interactive (i.e. receive pointer events) breaks the swipe down to close mechanism, since the inner document gets the pointer events and the story does not know about them.

In our shared example, amp-selector seems to be the service that is failing (ability to select a tab to toggle a new amp-iframe instance). In a separate issue/example we shared an example where the amp-selector properly toggles <img> but fails to toggle <amp-img>. Our thinking is that a solution to amp-selector will then work for the amp-img issue and the amp-iframe issue.

But I may be missing an important point you are making, so please redirect me if I'm off-base.

@newmuis
Copy link
Contributor

newmuis commented Dec 26, 2023

Answering your second question first:

In our shared example, amp-selector seems to be the service that is failing (ability to select a tab to toggle a new amp-iframe instance). In a separate issue/example we shared an example where the amp-selector properly toggles but fails to toggle . Our thinking is that a solution to amp-selector will then work for the amp-img issue and the amp-iframe issue.

Understood. The question isn't around the interactivity outside of the frame, but rather inside. If you want the user to be able to e.g. scroll the page inside the frame, or click on things inside the frame, that means that the AMP runtime needs to allow users to click on or interact with the iframe. But doing so will break the swipe down to close mechanism for the attachment drawer, unless the site inside the frame implements a pretty advanced communication layer, which seems unlikely.

We are loading content from sites we own/control - not 3rd party sites. As illustrated in example, amp-iframe works.
So we are hoping that the validator can be updated to accept the amp-iframe if it is part of story-attachment (descendent of that section).

Yes, and this makes sense. I think there's a philosophical question here, though: the AMP runtime cannot tell that you own this site, cannot tell whether the embedded page will interfere with the navigational paradigms, and cannot (AFAIK) tell whether the page successfully loads. Therefore, it cannot tell whether or not the user experience will be broken. The creator of the page might know all of these things, at least at the time that the page is published; that said, al of those things can change over time and it is unlikely that the creator will continue to test them in perpetuity.

With that, should the AMP runtime entrust the user experience to the creator, for a failure state that could potentially confuse the user and/or lead to broken navigation?

Previously, we decided not to. It's possible to revisit this decision, especially as browser technologies change. But this is the rationale.

@jaygray0919
Copy link
Author

OK, thanks for the explanation, I better understand.
We'll back down on this request.
Instead, in an attachment, we'll amp-img a target amp-iframe as a preview and then link to the preview.

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

No branches or pull requests

3 participants