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

Issue 26997002: Should paint border before background when bleedAvoidance is BackgroundBleedBackgroundOverBorder (Closed)

Created:
7 years, 2 months ago by tasak
Modified:
7 years ago
CC:
blink-reviews, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering
Visibility:
Public.

Description

Should paint border before background when bleedAvoidance is BackgroundBleedBackgroundOverBorder. This is a fixing patch for a regression caused by r158295: http://src.chromium.org/viewvc/blink?revision=158295&view=revision BUG=305216 TEST=fast/table/table-with-border-radius.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163220

Patch Set 1 #

Patch Set 2 : Revised #

Total comments: 8

Patch Set 3 : #

Total comments: 2

Patch Set 4 : Patch for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -11 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/table/table-with-border-radius.html View 1 2 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/table/table-with-border-radius-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
A + LayoutTests/platform/linux/fast/table/table-with-border-radius-expected.png View Binary file 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderTable.h View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/rendering/RenderTable.cpp View 1 2 3 1 chunk +0 lines, -9 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
tasak
7 years, 2 months ago (2013-10-11 09:19:16 UTC) #1
Julien - ping for review
I don't think I understand the fix. Why is not checking bleedAvoidance the right fix? ...
7 years, 2 months ago (2013-10-11 19:12:26 UTC) #2
tasak
On 2013/10/11 19:12:26, Julien Chaffraix wrote: > I don't think I understand the fix. Why ...
7 years, 2 months ago (2013-10-15 02:31:22 UTC) #3
tasak
I found a correct way to fix this issue. What I really need to do ...
7 years, 2 months ago (2013-10-15 03:47:35 UTC) #4
Julien - ping for review
Sorry for the delay, I missed your updated patch. https://codereview.chromium.org/26997002/diff/72001/LayoutTests/fast/table/table-with-border-radius.html File LayoutTests/fast/table/table-with-border-radius.html (right): https://codereview.chromium.org/26997002/diff/72001/LayoutTests/fast/table/table-with-border-radius.html#newcode25 LayoutTests/fast/table/table-with-border-radius.html:25: ...
7 years, 1 month ago (2013-10-25 00:29:15 UTC) #5
tasak
Thank you for reviewing. https://codereview.chromium.org/26997002/diff/72001/LayoutTests/fast/table/table-with-border-radius.html File LayoutTests/fast/table/table-with-border-radius.html (right): https://codereview.chromium.org/26997002/diff/72001/LayoutTests/fast/table/table-with-border-radius.html#newcode25 LayoutTests/fast/table/table-with-border-radius.html:25: <div class="description">Test for crbug.com/305216: Borders ...
7 years ago (2013-11-27 08:20:43 UTC) #6
Julien - ping for review
lgtm https://codereview.chromium.org/26997002/diff/142001/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/26997002/diff/142001/Source/core/rendering/RenderBox.cpp#newcode1349 Source/core/rendering/RenderBox.cpp:1349: if (bleedAvoidance != BackgroundBleedBackgroundOverBorder && (!style()->hasAppearance() || (!themePainted ...
7 years ago (2013-12-03 22:57:49 UTC) #7
tasak
Thank you for reviewing. https://codereview.chromium.org/26997002/diff/142001/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/26997002/diff/142001/Source/core/rendering/RenderBox.cpp#newcode1349 Source/core/rendering/RenderBox.cpp:1349: if (bleedAvoidance != BackgroundBleedBackgroundOverBorder && ...
7 years ago (2013-12-05 04:00:28 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tasak@google.com/26997002/182001
7 years ago (2013-12-05 04:00:38 UTC) #9
commit-bot: I haz the power
7 years ago (2013-12-05 08:08:08 UTC) #10
Message was sent while issue was closed.
Change committed as 163220

Powered by Google App Engine
This is Rietveld 408576698