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

Intent-to-Deprecate: amp-selector changing selection on low trust action #24443

Closed
sparhami opened this issue Sep 10, 2019 · 0 comments · Fixed by #24431
Closed

Intent-to-Deprecate: amp-selector changing selection on low trust action #24443

sparhami opened this issue Sep 10, 2019 · 0 comments · Fixed by #24431
Labels
Component: amp-selector Customer: Developer INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. P2: Soon Type: DevX issues impacting developer experience WG: components

Comments

@sparhami
Copy link

Also, it doesn't seem correct that amp-selector should be able to toggle via a low trust action when it doesn't have a size defined layout (layout="container"). This means any low trust action (such as page scrolling via position observer) can cause the page layout to jump, which should not be the case.

Contrast this with carousel, which allows you to change slides on low trust action, but requires a defined size, so this cannot cause the page to jump.

As far as I know, amp-selector has the only low trust action in AMP that can affect page layout. My understanding of action trust is that this should not be the case.

Originally posted by @sparhami in #24425 (comment)

We want to deprecate low trust actions (e.g. scroll via amp-position-observer, slideChange via carousel's autoplay, video playback ending, etc) being able to affect page layout via <amp-selector>. The amp-selector component allows use of layout="container" and coupled with the fact that it sets an attribute, means that it can be used to affect page layout.

For example, this could be used with a 1px tall carousel and autoplay to cause an ad to jump into the middle of a page using the autoplay delay. It could also be used with amp-position-observer to cause an add to jump in when the user scrolls a certain distance.

We can still allow the selection to change if the amp selector itself has a defined layout size (i.e. not using layout="container").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: amp-selector Customer: Developer INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. P2: Soon Type: DevX issues impacting developer experience WG: components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants