ampproject / amphtml

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

amp-ima-video bug on IOS devices: no player controls on content video if data-tag returns no ad #17961

Closed skarl-as closed 6 years ago

skarl-as commented 6 years ago

## What's the issue? amp-ima-video bug on IOS devices: no player controls (stop button) on content video if data-tag returns no ad

## How do we reproduce the issue? Example 1 - everything works as expected https://amp.welt.de/werbemitteltest2/article173084675/Ein-Artikel-mit-Video.html

Example 2 - missing controls https://amp.welt.de/werbemitteltest/article116430272/Ohne-Werbung-koennen-wir-das-Internet-zumachen.html

## What browsers are affected? bug only on iOS devices, e.g. iPhone 8 (iOS 11.4.1) Android devices work

## Which AMP version is affected? https://cdn.ampproject.org/v0/amp-ima-video-0.1.js

Implementation code on amp page:

<script async custom-element="amp-ima-video" src="https://cdn.ampproject.org/v0/amp-ima-video-0.1.js"></script>
...
<amp-ima-video width="16"
               height="9"
               layout="responsive"
               data-poster="https://www.welt.de/img/sport/mobile181418992/4172505727-ci102l-wWIDTH/SIDW-VD-20180904-SPO-DEU-LaLiga-SID50407W-TDE-de-jpg.jpg"
               data-tag="https://ib.adnxs.com/ptv?member=7823&amp;inv_code=welt.de-amp-werbemitteltest_video-preroll&amp;kw_vidContentId=181418898&amp;kw_misc=Erste-Bundesliga-%28Geo%3A-D%29%2CDFL-%28Deutsche-Fu%C3%9Fball-Liga%29%2CSeifert-Christian%2CPrimera-Division&amp;kw_vidWeltLocation=Opener&amp;pt2=www.welt.de&amp;pt3=welt">
        <source src="...
</amp-ima-video>
kevinkassimo commented 6 years ago

Hi @skarl-as . Thanks for the report! The primary reason for this issue is that the event for showing controls failed to be registered on ads error. For example, compare https://github.com/ampproject/amphtml/blob/bdd03bc2a6645f2d53db56b0ddf44ef493bb409f/ads/google/imaVideo.js#L718-L725 v.s. https://github.com/ampproject/amphtml/blob/bdd03bc2a6645f2d53db56b0ddf44ef493bb409f/ads/google/imaVideo.js#L647-L651 The latter failed to register the showControls handler.

Creating a PR for the fix.

ampprojectbot commented 6 years ago

This issue doesn't have a category which makes it harder for us to keep track of it. @aghassemi Please add an appropriate category.

skarl-as commented 6 years ago

@aghassemi @kevinkassimo Thanks guys... so as far as I understand we just have to wait for the next release