Summary: | Allow removal of white backgrounds | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||||||||||||||||||||||||||
Component: | New Bugs | Assignee: | Dean Jackson <dino> | ||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | ews-watchlist, jonlee, rniwa, ryanhaddad, simon.fraser, tsavell, webkit-bug-importer | ||||||||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Dean Jackson
2018-07-11 17:53:02 PDT
Created attachment 344803 [details]
Patch
Comment on attachment 344803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344803&action=review > Source/WebCore/rendering/InlineFlowBox.cpp:1365 > + if (renderer().document().settings().punchOutWhiteBackgrounds() && Color::isWhiteColor(color) && lineStyle.hasAppleColorFilter()) I think we may want this to be independent of color filter. Let's wait until we find out. Comment on attachment 344803 [details] Patch Attachment 344803 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8510387 New failing tests: displaylists/replay-skip-clipped-rect.html displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html css3/color-filters/punch-out-white-backgrounds.html displaylists/layer-dispay-list.html Created attachment 344806 [details]
Archive of layout-test-results from ews100 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 344803 [details] Patch Attachment 344803 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8510397 New failing tests: displaylists/replay-skip-clipped-rect.html displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html displaylists/layer-dispay-list.html Created attachment 344807 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 344803 [details] Patch Attachment 344803 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8510438 New failing tests: displaylists/replay-skip-clipped-rect.html displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html css3/color-filters/punch-out-white-backgrounds.html displaylists/layer-dispay-list.html Created attachment 344809 [details]
Archive of layout-test-results from ews117 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 344803 [details] Patch Attachment 344803 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8510697 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html Created attachment 344812 [details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 344850 [details]
Patch
Created attachment 344851 [details]
Patch
Created attachment 344855 [details]
Patch
Created attachment 344859 [details]
Patch
Created attachment 344862 [details]
Patch
Comment on attachment 344859 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344859&action=review > Source/WebCore/rendering/RenderTableCell.cpp:1291 > + if (color.isValid()) { Is it a problem that before we would color-filter regardless of whether the color is valid, but here we only filter if it is valid? And why do we not check the validity of the color prior to filtering in all of the other cases? Comment on attachment 344859 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344859&action=review >> Source/WebCore/rendering/RenderTableCell.cpp:1291 >> + if (color.isValid()) { > > Is it a problem that before we would color-filter regardless of whether the color is valid, but here we only filter if it is valid? And why do we not check the validity of the color prior to filtering in all of the other cases? This is a good point. isWhiteColor should check validity, as well as colorByApplyingColorFilter, so this code is not necessary. Comment on attachment 344862 [details] Patch Attachment 344862 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8517653 New failing tests: displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html Created attachment 344868 [details]
Archive of layout-test-results from ews101 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 344859 [details] Patch Attachment 344859 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8517455 New failing tests: displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html Created attachment 344871 [details]
Archive of layout-test-results from ews112 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 344862 [details] Patch Attachment 344862 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8517936 New failing tests: displaylists/extent-includes-shadow.html displaylists/extent-includes-transforms.html Created attachment 344873 [details]
Archive of layout-test-results from ews113 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 344983 [details]
Patch
Comment on attachment 344983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344983&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:952 > + context.fillRect(backgroundRectForPainting, bgColor, operation); // FIXME: use op? Do we need to fix? Created attachment 345125 [details]
Patch
Comment on attachment 345125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345125&action=review > Source/WebCore/rendering/InlineFlowBox.cpp:1367 > + WTFLogAlways("punching out inline"); Nope. > Source/WebCore/rendering/RenderBoxModelObject.cpp:952 > + context.fillRect(backgroundRectForPainting, bgColor, operation); // FIXME: use op if it is DestinationOut? Remove or fix the fixme. > Source/WebCore/rendering/RenderTableCell.cpp:1293 > + WTFLogAlways("punching out table cell"); Nope. > Source/WebCore/rendering/RenderTheme.h:256 > + virtual bool usingDarkAppearance(const Page&) const { return false; } Can RenderTheme really not get to Page()? Maybe just pass the renderer? > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:173 > + if (options.punchOutWhiteBackgroundsInDarkMode) > + m_mainWebView->setDrawsBackground(false); Hmmmm, weird. Those things naively seem unrelated. You also need to reset this between tests. Committed r233869: <https://trac.webkit.org/changeset/233869> Looks like we have two failing tests after r233869 <https://trac.webkit.org/changeset/233869/webkit> Test Histories: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=displaylists%2Fextent-includes-transforms.html%20displaylists%2Fextent-includes-shadow.html Test Diff: --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/displaylists/extent-includes-shadow-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/displaylists/extent-includes-shadow-actual.txt @@ -10,8 +10,10 @@ (shadow-offset width=10 height=20) (shadows-use-legacy-radius 0) (should-subpixel-quantize-fonts 0)) -(fill-rect-with-color +(fill-composited-rect (extent at (43,50) size 134x137) (rect at (50,50) size 100x100) - (color #0000FF)) + (color #0000FF) + (composite-operation source-over) + (blend-mode normal)) (restore) (In reply to Truitt Savell from comment #30) > Looks like we have two failing tests after r233869 > <https://trac.webkit.org/changeset/233869/webkit> I went ahead and rebaselined these tests for mac-wk1 in https://trac.webkit.org/r233887 |