Bug 209590

Summary: REGRESSION (r254669): Expand media button doesn't work on first try on photos on reddit.com
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cathiechen, darin, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201641
https://bugs.webkit.org/show_bug.cgi?id=224664
Attachments:
Description Flags
patch
none
patch
none
patch none

Antti Koivisto
Reported 2020-03-26 03:30:55 PDT
1. Visit old.reddit.com 2. Click the play button that expands an inline media preview 3. A lot of the time photos don’t appear 4. Click the x to collapse the non-visible preview 5. Click the play button again and the preview appears
Attachments
patch (3.75 KB, patch)
2020-03-26 03:58 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (5.91 KB, patch)
2020-03-26 06:58 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (5.90 KB, patch)
2020-03-26 07:32 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2020-03-26 03:31:19 PDT
<rdar://problem/60461809>
Antti Koivisto
Comment 2 2020-03-26 03:58:56 PDT
Created attachment 394591 [details] patch
Antti Koivisto
Comment 3 2020-03-26 06:58:57 PDT
Created attachment 394602 [details] patch
Antti Koivisto
Comment 4 2020-03-26 07:32:38 PDT
Created attachment 394606 [details] patch
Darin Adler
Comment 5 2020-03-26 10:28:42 PDT
Comment on attachment 394606 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=394606&action=review > Source/WebCore/rendering/RenderReplaced.cpp:597 > + // For images with explicit width/height this updates the instrinsic size as a side effect. > + computeAspectRatioInformationForRenderBox(embeddedContentBox(), constrainedSize, intrinsicRatio); Seems obviously a good fix for now. Are we happy with this side effect relationship here for the longer term? Also, what guarantees embeddedContentBox() is not nullptr?
Antti Koivisto
Comment 6 2020-03-26 10:39:38 PDT
> Are we happy with this side effect relationship here for the longer term? Not really, but in the catastrophe that is the current render tree it is a pretty minor detail. > Also, what guarantees embeddedContentBox() is not nullptr? It is a valid (and the common) case for it to be nullptr.
EWS
Comment 7 2020-03-26 10:44:51 PDT
Committed r259051: <https://trac.webkit.org/changeset/259051> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394606 [details].
cathiechen
Comment 8 2021-04-16 03:34:03 PDT
Comment on attachment 394606 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=394606&action=review > LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt:3 > +FAIL Image width and height attributes are used to infer aspect-ratio assert_approx_equals: expected 4 +/- 0.001 but got 4.166666666666667 Hmmm, this failure blocks all the cases below...
Antti Koivisto
Comment 9 2021-04-16 05:31:41 PDT
Like the ChangeLog says "Failure here shifts to a different subtest. This one uses fractional pixels and LayoutUnit accuracy is not sufficient to compute the exact ratio." It is a badly constructed test. It should be changed so that individual tests generate PASSes independently.
cathiechen
Comment 10 2021-04-16 05:59:21 PDT
(In reply to Antti Koivisto from comment #9) > Like the ChangeLog says > > "Failure here shifts to a different subtest. This one uses fractional pixels > and LayoutUnit accuracy is not sufficient to compute the exact ratio." > > It is a badly constructed test. It should be changed so that individual > tests generate PASSes independently. Yes, agree... I'm trying to upload a patch to work around these in https://bugs.webkit.org/show_bug.cgi?id=224664
Note You need to log in before you can comment on or make changes to this bug.