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

Issue 1201753003: Apply outline-offset on all edges (not just top/left) (Closed)

Created:
5 years, 6 months ago by fs
Modified:
5 years, 6 months ago
Reviewers:
chrishtr, f(malita)
CC:
blink-reviews, blink-reviews-paint_chromium.org, dshwang, slimming-paint-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Apply outline-offset on all edges (not just top/left) In InlinePainter::paintOutlineForLine, the "outline rectangles" are formed by expanding and shifting by outline-offset. This however only result in an offset of the outline on the top and left sides. Rewrite the code to use an inflate() operation instead, so that all four edges are affected by the offset. The spec describes the effect of the 'outline-offset' property as: "If the computed value of outline-offset is anything other than 0, then the outline is outset from the border edge by that amount." "Negative values must cause the outline to shrink into the border box." (http://dev.w3.org/csswg/css-ui-3/#outline-offset) This is also what ObjectPainter::paintOutline does. This makes us match Gecko more closely rendering-wise. TEST=fast/inline/outline-offset.html (S.P. strict cull rect) BUG=495368 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197589

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -6 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +3 lines, -0 lines 0 comments Download
A LayoutTests/paint/inline/outline-offset.html View 1 chunk +22 lines, -0 lines 0 comments Download
M Source/core/paint/InlinePainter.cpp View 2 chunks +11 lines, -6 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
fs
Not a whole lot of test-coverage for this, so I added a simple rendering test. ...
5 years, 6 months ago (2015-06-22 13:32:26 UTC) #2
chrishtr
lgtm
5 years, 6 months ago (2015-06-22 18:19:41 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1201753003/1
5 years, 6 months ago (2015-06-22 18:20:11 UTC) #5
commit-bot: I haz the power
5 years, 6 months ago (2015-06-22 18:24:44 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197589

Powered by Google App Engine
This is Rietveld 408576698