RESOLVED FIXED203642
covscan: Incorrect check in RenderBox::hasOverrideContainingBlockContentHeight
https://bugs.webkit.org/show_bug.cgi?id=203642
Summary covscan: Incorrect check in RenderBox::hasOverrideContainingBlockContentHeight
Eike Rathke
Reported 2019-10-30 16:20:11 PDT
Fix covscan identical branches
Attachments
Patch (1.78 KB, patch)
2019-10-30 16:33 PDT, Eike Rathke
ews-watchlist: commit-queue-
Archive of layout-test-results from ews211 for win-future (14.43 MB, application/zip)
2019-10-31 09:12 PDT, EWS Watchlist
no flags
Eike Rathke
Comment 1 2019-10-30 16:33:13 PDT
Alexey Proskuryakov
Comment 2 2019-10-30 23:29:04 PDT
Comment on attachment 382388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382388&action=review > Source/WebCore/ChangeLog:11 > + Checking HeightMap twice is definitely wrong. > + Given that hasOverrideContainingBlockContentHeight() should > + do the opposite of hasOverrideContainingBlockContentWidth() and > + correspond with overrideContainingBlockContentHeight(), do so. Since this is a bug fix, it should come with a regression test that fails before the fix, and passes after the fix.
EWS Watchlist
Comment 3 2019-10-31 09:12:13 PDT
Comment on attachment 382388 [details] Patch Attachment 382388 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13195733 New failing tests: fast/repaint/backgroundSizeRepaint.html
EWS Watchlist
Comment 4 2019-10-31 09:12:15 PDT
Created attachment 382465 [details] Archive of layout-test-results from ews211 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews211 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Eike Rathke
Comment 5 2019-11-01 09:24:52 PDT
If the test now fails due to this change it means that cb->style().isHorizontalWritingMode() is false there. Really intended? i.e. does the tested code actually set that to false on purpose? All other places that ask isHorizontalWritingMode() already use X/width for width if true, and Y/height if false (and vice versa for height), not doing it here looks unexpected to me. And then no check on isHorizontalWritingMode() at all would be needed. The only place calling RenderBox::hasOverrideContainingBlockContentHeight() seems to be RenderBoxModelObject::relativePositionOffset() I'm off here.
Rob Buis
Comment 6 2021-04-30 09:27:33 PDT
Note You need to log in before you can comment on or make changes to this bug.