Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(94)

Issue 208393004: Fix zero height root renderer with background image painting (Closed)

Created:
6 years, 9 months ago by leviw_travelin_and_unemployed
Modified:
6 years, 9 months ago
Reviewers:
ojan, esprehn, eseidel
CC:
blink-reviews, bemjb+rendering_chromium.org, dsinclair, zoltan1, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, pdr., rune+blink
Visibility:
Public.

Description

Fix zero height root renderer with background image painting When the root renderer is fully collapsed with a background image, the solid color background painting path would assume that the background image would fill the viewport, but the background image code would, correctly, only use the actual height of the root and therefor not fill anything. The result is the viewport isn't filled at all. This is a regression from my early patch (r166582) that avoids implicitly painting the viewport background when the root itself will do so as an optimization to avoid rasterizing the entire viewport twice. Catching the case in the solid color background painting path. Behavior matches FireFox and Safari. BUG=349936 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169889

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address Elliott's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -0 lines) Patch
A LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height.html View 1 1 chunk +8 lines, -0 lines 0 comments Download
A + LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html View 1 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
leviw_travelin_and_unemployed
6 years, 9 months ago (2014-03-21 19:48:41 UTC) #1
leviw_travelin_and_unemployed
Ping
6 years, 9 months ago (2014-03-24 01:18:37 UTC) #2
esprehn
lgtm, I don't entirely understand this stuff, but it seems okay.
6 years, 9 months ago (2014-03-24 01:32:31 UTC) #3
esprehn
https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html File LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html (right): https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html#newcode6 LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html:6: </body> Why delete this? https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height.html File LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height.html (right): https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height.html#newcode3 ...
6 years, 9 months ago (2014-03-24 01:32:39 UTC) #4
leviw_travelin_and_unemployed
https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html File LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html (right): https://codereview.chromium.org/208393004/diff/1/LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html#newcode6 LayoutTests/fast/backgrounds/root-with-generated-background-and-no-height-expected.html:6: </body> On 2014/03/24 01:32:40, esprehn wrote: > Why delete ...
6 years, 9 months ago (2014-03-24 17:06:48 UTC) #5
leviw_travelin_and_unemployed
The CQ bit was checked by leviw@chromium.org
6 years, 9 months ago (2014-03-24 21:50:36 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/leviw@chromium.org/208393004/20001
6 years, 9 months ago (2014-03-24 21:50:41 UTC) #7
commit-bot: I haz the power
6 years, 9 months ago (2014-03-24 23:51:54 UTC) #8
Message was sent while issue was closed.
Change committed as 169889

Powered by Google App Engine
This is Rietveld 408576698