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 Rendering | Assignee: | 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
Antti Koivisto
2020-03-26 03:30:55 PDT
Created attachment 394591 [details]
patch
Created attachment 394602 [details]
patch
Created attachment 394606 [details]
patch
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? > 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. Committed r259051: <https://trac.webkit.org/changeset/259051> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394606 [details]. 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... 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. (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 |