Bug 153299

Summary: Outline should contribute to visual overflow.
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: hyatt, mcatanzaro, mmaxfield, mvujovic, simon.fraser
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews116 for mac-yosemite
none
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite
none
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
outline auto repaint issues
none
outline auto repaint issues
none
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews113 for mac-yosemite
none
Patch hyatt: review+

zalan
Reported 2016-01-20 19:24:07 PST
RenderView::setMaximalOutlineSize is evil.
Attachments
Patch (65.14 KB, patch)
2016-01-20 21:14 PST, zalan
no flags
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.07 MB, application/zip)
2016-01-20 21:52 PST, Build Bot
no flags
Archive of layout-test-results from ews103 for mac-yosemite (814.46 KB, application/zip)
2016-01-20 22:06 PST, Build Bot
no flags
Archive of layout-test-results from ews116 for mac-yosemite (1.04 MB, application/zip)
2016-01-20 22:16 PST, Build Bot
no flags
Patch (68.56 KB, patch)
2016-01-21 14:19 PST, zalan
no flags
Patch (63.04 KB, patch)
2016-01-21 14:30 PST, zalan
buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite (786.99 KB, application/zip)
2016-01-21 15:32 PST, Build Bot
no flags
Archive of layout-test-results from ews101 for mac-yosemite (744.71 KB, application/zip)
2016-01-21 15:56 PST, Build Bot
no flags
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (877.33 KB, application/zip)
2016-01-21 18:21 PST, Build Bot
no flags
outline auto repaint issues (1.44 KB, text/html)
2016-01-28 13:11 PST, zalan
no flags
outline auto repaint issues (1.54 KB, text/html)
2016-02-01 13:46 PST, zalan
no flags
Patch (81.31 KB, patch)
2016-02-02 15:12 PST, zalan
no flags
Patch (86.54 KB, patch)
2016-02-03 10:02 PST, zalan
no flags
Patch (87.16 KB, patch)
2016-02-03 21:07 PST, zalan
no flags
Patch (86.91 KB, patch)
2016-02-03 21:25 PST, zalan
no flags
Archive of layout-test-results from ews113 for mac-yosemite (793.20 KB, application/zip)
2016-02-03 22:22 PST, Build Bot
no flags
Patch (86.91 KB, patch)
2016-02-04 08:39 PST, zalan
hyatt: review+
zalan
Comment 1 2016-01-20 20:52:43 PST
Simon Fraser (smfr)
Comment 2 2016-01-20 21:00:46 PST
Exciting!
zalan
Comment 3 2016-01-20 21:14:18 PST
zalan
Comment 4 2016-01-20 21:14:37 PST
Comment on attachment 269428 [details] Patch WIP patch.
Build Bot
Comment 5 2016-01-20 21:52:14 PST
Comment on attachment 269428 [details] Patch Attachment 269428 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/720117 New failing tests: fast/multicol/multicol-with-child-renderLayer-for-input.html accessibility/mac/selection-element-tabbing-to-link.html fast/repaint/4776765.html accessibility/mac/selection-notification-focus-change.html
Build Bot
Comment 6 2016-01-20 21:52:17 PST
Created attachment 269430 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2016-01-20 22:06:29 PST
Comment on attachment 269428 [details] Patch Attachment 269428 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/720156 New failing tests: fast/multicol/multicol-with-child-renderLayer-for-input.html accessibility/mac/selection-element-tabbing-to-link.html fast/repaint/4776765.html accessibility/mac/selection-notification-focus-change.html
Build Bot
Comment 8 2016-01-20 22:06:32 PST
Created attachment 269432 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 9 2016-01-20 22:16:39 PST
Comment on attachment 269428 [details] Patch Attachment 269428 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/720157 New failing tests: imported/blink/fast/forms/select/styling-option-checked.html fast/multicol/multicol-with-child-renderLayer-for-input.html accessibility/mac/selection-element-tabbing-to-link.html imported/blink/fast/html/crash-on-invalid-selection-index.html fast/forms/select/selectall-command-crash.html fast/repaint/4776765.html accessibility/mac/selection-notification-focus-change.html
Build Bot
Comment 10 2016-01-20 22:16:43 PST
Created attachment 269433 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
zalan
Comment 11 2016-01-21 14:19:11 PST
zalan
Comment 12 2016-01-21 14:23:37 PST
Remaining issues: 1. multi-col clips outline now that it's part of visual overflow. Non-multi-column nested layer painting works (no clipping) see: fast/multicol/multicol-with-child-renderLayer-for-input.html (and somewhat related: imported/blink/fast/multicol/outlines-at-column-boundaries.html) 2. Have someone fixed that PostResolutionCallbackDisabler issue (not a blocker)
zalan
Comment 13 2016-01-21 14:30:29 PST
Build Bot
Comment 14 2016-01-21 15:32:21 PST
Comment on attachment 269493 [details] Patch Attachment 269493 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/723069 New failing tests: fast/repaint/4776765.html
Build Bot
Comment 15 2016-01-21 15:32:25 PST
Created attachment 269506 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 16 2016-01-21 15:56:35 PST
Comment on attachment 269493 [details] Patch Attachment 269493 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/723196 New failing tests: fast/repaint/4776765.html
Build Bot
Comment 17 2016-01-21 15:56:39 PST
Created attachment 269509 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 18 2016-01-21 18:21:18 PST
Comment on attachment 269493 [details] Patch Attachment 269493 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/723452 New failing tests: fast/repaint/4776765.html
Build Bot
Comment 19 2016-01-21 18:21:22 PST
Created attachment 269530 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
zalan
Comment 20 2016-01-28 13:11:33 PST
Created attachment 270142 [details] outline auto repaint issues
zalan
Comment 21 2016-02-01 13:46:03 PST
Created attachment 270420 [details] outline auto repaint issues
zalan
Comment 22 2016-02-01 14:35:19 PST
*** Bug 104460 has been marked as a duplicate of this bug. ***
zalan
Comment 23 2016-02-02 15:12:13 PST
zalan
Comment 24 2016-02-03 10:02:53 PST
Dave Hyatt
Comment 25 2016-02-03 11:12:09 PST
Comment on attachment 270582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270582&action=review Comments in bug. > Source/WebCore/rendering/InlineFlowBox.cpp:955 > + if (!lineStyle.hasOutline()) > + return; You should check lineStyle.outlineSize() too, since outlineWidth() can be positive, but outlineOffset pushes you back in. If outlineSize() == 0, then you don't have overflow, even if outlineWidth() > 0. > Source/WebCore/rendering/RenderBox.cpp:4496 > + if (!style().boxShadow() && !style().hasBorderImageOutsets() && !outlineStyleForRepaint().hasOutline()) Same problem here. Should check outlineSize(). > Source/WebCore/rendering/RenderBox.cpp:4543 > + if (outlineStyleForRepaint().hasOutline()) { Same. > Source/WebCore/rendering/style/RenderStyle.cpp:2034 > + if (outlineStyleIsAuto()) > + return RenderTheme::platformFocusRingWidth(); This does not seem right to me. If outline-style is auto, you are ignoring the actual parsed value of outline width? Won't this cause us not to honor outline width at all? > Source/WebCore/rendering/style/RenderStyle.h:671 > + float outlineWidth() const; > + bool hasOutline() const { return outlineStyle() > BHIDDEN && outlineWidth() > 0; } You might want to make a helper like "bool hasOutlineInVisualOverflow() const { return hasOutline() && outlineSize() > 0; }" and use it in the places I mentioned earlier. Consider making some tests with negative outline offsets that pull the ring back in.
zalan
Comment 26 2016-02-03 21:07:21 PST
zalan
Comment 27 2016-02-03 21:25:57 PST
Build Bot
Comment 28 2016-02-03 22:22:29 PST
Comment on attachment 270628 [details] Patch Attachment 270628 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/780117 New failing tests: imported/w3c/web-platform-tests/streams-api/readable-streams/garbage-collection-2.html
Build Bot
Comment 29 2016-02-03 22:22:32 PST
Created attachment 270635 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
zalan
Comment 30 2016-02-04 08:37:20 PST
(In reply to comment #28) > Comment on attachment 270628 [details] > Patch > > Attachment 270628 [details] did not pass mac-debug-ews (mac): > Output: http://webkit-queues.webkit.org/results/780117 > > New failing tests: > imported/w3c/web-platform-tests/streams-api/readable-streams/garbage- > collection-2.html This failure looks completely unrelated.
zalan
Comment 31 2016-02-04 08:39:37 PST
Dave Hyatt
Comment 32 2016-02-04 09:12:32 PST
Comment on attachment 270658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270658&action=review r=me > Source/WebCore/rendering/style/RenderStyle.cpp:483 > + if (hasOutline() != other.hasOutline() > + || (hasOutline() > + && (outlineSize() != other.outlineSize()))) > + return true; Assuming I understand correctly, and this method should only return true if your visual overflow could change, then couldn't this check be stricter and use hasOutlineInVisualOverflow?
zalan
Comment 33 2016-02-06 15:07:58 PST
Michael Catanzaro
Comment 34 2016-02-06 18:18:30 PST
Zalan could you please sanity-check my follow-up in r196235: <http://trac.webkit.org/changeset/196235> Also, this caused bug #153956, any ideas about that?
zalan
Comment 35 2016-02-06 19:24:34 PST
(In reply to comment #34) > Zalan could you please sanity-check my follow-up in r196235: > <http://trac.webkit.org/changeset/196235> > > Also, this caused bug #153956, any ideas about that? I was going to ask cfleizach about it since there are some failures on Mac too. Changeset looks good. Thanks for taking care of that.
Ryosuke Niwa
Comment 36 2022-07-26 10:13:24 PDT
*** Bug 106397 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.