REGRESSION (r258215): Title preview movie isn't displayed at www.thismmalife.com

• Component: Images
• Status: RESOLVED FIXED

[Fernando]

This was tested on iOS 14.0 (18A5351d) beta 5

Browsing a website that presents the following GIF we're not able to see anything other than the background color of the element

https://res.cloudinary.com/hkyugldxm/image/fetch/s--tWG33PgR--/c_imagga_scale,f_auto,fl_progressive,h_420,q_66,w_1000/https://thismmalife-images.s3.amazonaws.com/i/nzb6iyu6cpodsele1fqe.gif

The `img` is presented in HTML as:

<img src="https://res.cloudinary.com/hkyugldxm/image/fetch/s--tWG33PgR--/c_imagga_scale,f_auto,fl_progressive,h_420,q_66,w_1000/https://thismmalife-images.s3.amazonaws.com/i/nzb6iyu6cpodsele1fqe.gif" width="1000" height="420" style="background-color:#dddddd;" class="crayons-article__cover__image" alt="Cover image for BJ Penn deals with GSP's low kicks like they are nothing 🤷🏻">

This is not a problem in iOS 13.6.1 (17G80). Could this be a regression on GIF support?

The web page that has this GIF also contains another GIF that is presented correctly, which makes the problem a bit more odd. It can be seen here: 

https://www.thismmalife.com/taliashaw_962/a-rose-namajunas-flying-arm-bar-move-4pmo

Thank you in advance for the help.

[Fernando]

I apologize, the correct URL for the article in the website that contains both the GIF that correctly renders and the one that doesn't render is the following:

https://www.thismmalife.com/leewynne/bj-penn-deals-with-gsp-s-low-kicks-like-they-are-nothing-42hc

[Alexey Proskuryakov]

This is also a Safari 14 regression on macOS.

The one that doesn't play is above the title. It's not actually a .gif despite what the URL says - it's an mp4 in Safari, and a webp in Chrome, loaded via an <img>.

[Radar WebKit Bug Importer]

<rdar://problem/67707957>

[Jer Noble]

Looks like the .mp4 in question has an output PTS delta applied, which leads to us grabbing the wrong sample out of the SampleMap and causing a decode error. Simple fix; just need to ask for the `CMSampleBufferGetOutputPresentationTimeStamp()` rather than the `CMSampleBufferGetPresentationTimeStamp()` in `ImageDecoderAVFObjC::storeSampleBuffer()` and write a test.

[Jer Noble]

Created attachment 407327 [details]
Patch

[Peng Liu]

Comment on attachment 407327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407327&action=review

> LayoutTests/media/video-as-img-output-pts-expected.txt:1
> +Tests that an  with a .mp4 source where that .mp4 has an edit list will correctly decode.

Something is missing here?

[Eric Carlson]

Comment on attachment 407327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407327&action=review

> LayoutTests/media/video-as-img-output-pts.html:8
> +        consoleWrite('Tests that an <img> with a .mp4 source where that .mp4 has an edit list will correctly decode.')

s/<img>/&lt;img&gt;/

[Jer Noble]

Created attachment 407335 [details]
Patch for landing

[Jer Noble]

Created attachment 407340 [details]
Patch for landing

[EWS]

Committed r266239: <https://trac.webkit.org/changeset/266239>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407335 [details].

[Fernando]

Hello again, was this patch shipped in the latest iOS beta release? 

I was still able to reproduce the problem but I haven't tested Safari preview release on macOS so I'm not sure if it's fixed there.

[Alexey Proskuryakov]

(In reply to Fernando from comment #11)
> Hello again, was this patch shipped in the latest iOS beta release? 

It was not.

> I was still able to reproduce the problem but I haven't tested Safari preview release on macOS so I'm not sure if it's fixed there.

Latest Safari 14 beta and Safari Technology Preview do include this fix.

[Alexey Proskuryakov]

> Latest Safari 14 beta and Safari Technology Preview do include this fix.

I was incorrect, they do not. STP 113 includes r265893, which is older, and beta 5 is older too.

[Alexey Proskuryakov]

*** Bug 216088 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

Further correction: latest Safari beta does have the fix, it’s STP that’s older.

[Colin Bendell]

Can you confirm that this patch landed in ios14 GM? If it did then we still have a regression.

[Fernando]

Same here, Safari in iOS 14.0 (18A373) still experiences the problem

[Jer Noble]

(In reply to Colin Bendell from comment #16)
> Can you confirm that this patch landed in ios14 GM? If it did then we still
> have a regression.

No, this change missed the cutoff. I'll ping this bug when a fix is publicly testable.

[Colin Bendell]

confirmed fixed in ios14.2 beta 1.

Aside: this is an example of why UA sniffing is still needed. While not ideal,  having browser version numbers in the UA allow service providers adapt content to work around bugs.

[Ahmad Saleem]

Landed - https://github.com/WebKit/WebKit/commit/94ce1012431e0b2648520fdace3a4369422d4937

and didn't backed out.