search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

🐛 Fix amp-story-player panel next/prev button in Chrome #40124

Closed ychsieh closed 3 weeks ago

ychsieh commented 2 months ago

40119

I suppose this is more of a temporary bug in Chrome. But at the same time I don't think we need these rules to hide buttons for other non-panel modes, because full-bleed should be the only non-panel mode(which is already included this rule set).

Adding @processprocess as the contributor of these rules to make sure I don't miss anything.

Updated: It turns out to be ancestor selector issue: :not(.i-amphtml-story-player-panel) .i-amphtml-story-player-panel-prev would also select .i-amphtml-story-player-panel-prev even though the immediate parent has .i-amphtml-story-player-panel so the button has opacity 0. This is because it's also directly within the tag, which does not have .i-amphtml-story-player-panel. Changing the ancestor selector to child selector solves the issue.

As for the reason why this previously works on other browsers like Safari and Firefox, my theory is that Chrome traces the ancestors all the way out of shadow root, e.g. , whereas Safari and Firefox don't. This means for this specific DOM tree in Safari and Firefox, ancestor and child selector has the same effect as the only ancestor is the parent, e.g. root element under shadow root.

processprocess commented 1 month ago

This makes the buttons appear when they shouldn't. For this reason I don't think we should merge this change.

This can be seen in the examples when running the dev server: /examples/amp-story/player-amp-desktop-panels.html /examples/amp-story/player-with-button.html /examples/amp-story/player.html /examples/amp-story/player-local-stories.html

Screenshot 2024-10-01 at 4 01 26 PM
ychsieh commented 1 month ago

It turns out to be ancestor selector issue: :not(.i-amphtml-story-player-panel) .i-amphtml-story-player-panel-prev would also select .i-amphtml-story-player-panel-prev even though the immediate parent has .i-amphtml-story-player-panel so the button has opacity 0. This is because it's also directly within the tag, which does not have .i-amphtml-story-player-panel. Changing the ancestor selector to child selector solves the issue.

As for the reason why this previously works on other browsers like Safari and Firefox, my theory is that Chrome traces the ancestors all the way out of shadow root, e.g. , whereas Safari and Firefox don't. This means for this specific DOM tree in Safari and Firefox, ancestor and child selector has the same effect as the only ancestor is the parent, e.g. root element under shadow root.