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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1269123002: Preparation for combining paths of focus rings and outlines (Closed)

Created:
5 years, 4 months ago by Xianzhu
Modified:
5 years, 4 months ago
CC:
aboxhall, blink-reviews, blink-reviews-paint_chromium.org, blink-reviews-rendering, blink-reviews-style_chromium.org, Rik, danakj, dmazzoni, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, f(malita), fs, gyuyoung2, jbroman, jchaffraix+rendering, je_julie, Justin Novosad, kouhei+svg_chromium.org, leviw+renderwatch, nektarios, pdr+svgwatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney, slimming-paint-reviews_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Preparation for combining paths of focus rings and outlines This is split from https://codereview.chromium.org/1224933002/ to ease review. - Rename addFocusRingRects to addOutlineRects; - Remove ComputedStyle::outlineSize() which returns incorrect value when outline style is 'auto'. We should use ComputedStyle::outlineOutsetExtent() (renamed from outlineOutset() to avoid ambiguity) instead. BUG=506669 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200049

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Rebaseline tests #

Patch Set 4 : #

Patch Set 5 : Remove debug from fast/css/focus-ring-recursive-continuations.html #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -118 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 2 chunks +5 lines, -1 line 0 comments Download
M LayoutTests/fast/css/focus-ring-recursive-continuations.html View 1 2 3 4 1 chunk +6 lines, -5 lines 2 comments Download
M LayoutTests/fast/css/focus-ring-recursive-continuations-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/css/focus-ring-recursive-inlines.html View 1 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/css/focus-ring-recursive-inlines-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/focus-continuations-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/focus-enable-continuations-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/inline-focus-expected.txt View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt View 1 2 3 2 chunks +2 lines, -6 lines 3 comments Download
M Source/core/layout/LayoutBlock.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/LayoutBlock.cpp View 2 chunks +6 lines, -6 lines 0 comments Download
M Source/core/layout/LayoutBox.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/LayoutBox.cpp View 1 2 2 chunks +11 lines, -12 lines 0 comments Download
M Source/core/layout/LayoutBoxModelObject.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/layout/LayoutBoxModelObject.cpp View 1 2 3 2 chunks +12 lines, -12 lines 0 comments Download
M Source/core/layout/LayoutInline.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/LayoutInline.cpp View 1 3 chunks +9 lines, -9 lines 0 comments Download
M Source/core/layout/LayoutObject.h View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/layout/LayoutObject.cpp View 1 2 3 2 chunks +4 lines, -3 lines 0 comments Download
M Source/core/layout/LayoutTableCell.cpp View 1 1 chunk +5 lines, -5 lines 0 comments Download
M Source/core/layout/LayoutTextControl.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/LayoutTextControl.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/line/InlineFlowBox.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGContainer.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGContainer.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGImage.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGImage.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGModelObject.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGModelObject.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/layout/svg/LayoutSVGShape.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/LayoutSVGShape.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/SVGLayoutSupport.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/paint/InlinePainter.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/paint/ObjectPainter.cpp View 1 3 chunks +5 lines, -5 lines 0 comments Download
M Source/core/style/ComputedStyle.h View 1 2 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/style/ComputedStyle.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/testing/Internals.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/testing/Internals.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/testing/Internals.idl View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/accessibility/AXLayoutObject.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/accessibility/AXSpinButton.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/graphics/GraphicsContext.h View 1 2 chunks +11 lines, -5 lines 0 comments Download
M Source/platform/graphics/GraphicsContext.cpp View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/platform/graphics/PaintInvalidationReason.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/graphics/PaintInvalidationReason.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/platform/graphics/skia/SkiaUtils.cpp View 1 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 16 (4 generated)
Xianzhu
5 years, 4 months ago (2015-08-03 22:05:35 UTC) #2
chrishtr
https://codereview.chromium.org/1269123002/diff/20001/Source/core/layout/LayoutBox.cpp File Source/core/layout/LayoutBox.cpp (left): https://codereview.chromium.org/1269123002/diff/20001/Source/core/layout/LayoutBox.cpp#oldcode4125 Source/core/layout/LayoutBox.cpp:4125: LayoutUnit outlineSize = style()->outlineSize(); The functionality changes involving fixing ...
5 years, 4 months ago (2015-08-04 05:09:02 UTC) #3
Xianzhu
https://codereview.chromium.org/1269123002/diff/20001/Source/core/layout/LayoutBox.cpp File Source/core/layout/LayoutBox.cpp (left): https://codereview.chromium.org/1269123002/diff/20001/Source/core/layout/LayoutBox.cpp#oldcode4125 Source/core/layout/LayoutBox.cpp:4125: LayoutUnit outlineSize = style()->outlineSize(); On 2015/08/04 05:09:02, chrishtr wrote: ...
5 years, 4 months ago (2015-08-04 17:41:42 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1269123002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1269123002/40001
5 years, 4 months ago (2015-08-04 21:23:54 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/65029)
5 years, 4 months ago (2015-08-04 22:59:13 UTC) #8
chrishtr
https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/fast/css/focus-ring-recursive-continuations.html File LayoutTests/fast/css/focus-ring-recursive-continuations.html (right): https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/fast/css/focus-ring-recursive-continuations.html#newcode15 LayoutTests/fast/css/focus-ring-recursive-continuations.html:15: shouldBe("internals.outlineRects(document.getElementById('focus')).length", "6"); Why are there now 6? Was this ...
5 years, 4 months ago (2015-08-05 03:57:50 UTC) #9
Xianzhu
https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/fast/css/focus-ring-recursive-continuations.html File LayoutTests/fast/css/focus-ring-recursive-continuations.html (right): https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/fast/css/focus-ring-recursive-continuations.html#newcode15 LayoutTests/fast/css/focus-ring-recursive-continuations.html:15: shouldBe("internals.outlineRects(document.getElementById('focus')).length", "6"); On 2015/08/05 03:57:50, chrishtr wrote: > Why ...
5 years, 4 months ago (2015-08-05 15:58:43 UTC) #10
chrishtr
https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt File LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt (left): https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt#oldcode22 LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt:22: "LayoutBlockFlow DIV class='linkbar'", On 2015/08/05 at 15:58:43, Xianzhu wrote: ...
5 years, 4 months ago (2015-08-05 16:53:59 UTC) #11
Xianzhu
On 2015/08/05 16:53:59, chrishtr wrote: > https://codereview.chromium.org/1269123002/diff/80001/LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt > File > LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt > (left): > > ...
5 years, 4 months ago (2015-08-05 17:07:49 UTC) #12
chrishtr
lgtm
5 years, 4 months ago (2015-08-05 17:11:08 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1269123002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1269123002/80001
5 years, 4 months ago (2015-08-05 17:11:35 UTC) #15
commit-bot: I haz the power
5 years, 4 months ago (2015-08-05 17:15:38 UTC) #16
Message was sent while issue was closed. 
Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200049

Powered by Google App Engine
This is Rietveld 408576698