Bug 187072

Summary:

Find on page selection color isn't adapted for dark mode

Product:

WebKit

Reporter:

Timothy Hatcher <timothy>

Component:

Layout and Rendering

Assignee:

Timothy Hatcher <timothy>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue, dbates, ews-watchlist, megan_gardner, rniwa, ryanhaddad, simon.fraser, thorton, tsavell, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews100 for mac-sierra	none
Archive of layout-test-results from ews117 for mac-sierra	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from webkit-cq-03 for mac-sierra	none
Patch	none
Patch	none
Archive of layout-test-results from webkit-cq-01 for mac-sierra	none
Patch	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews103 for mac-sierra	none
Archive of layout-test-results from ews115 for mac-sierra	none
Patch	none

Timothy Hatcher

Reported 2018-06-26 17:13:22 PDT

White text on a yellow background is hard to read. <rdar://problem/40354841>

Attachments
Patch (16.64 KB, patch) 2018-06-26 17:35 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (16.74 KB, patch) 2018-06-26 18:33 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-sierra (2.41 MB, application/zip) 2018-06-26 19:41 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews117 for mac-sierra (3.19 MB, application/zip) 2018-06-26 20:16 PDT, EWS Watchlist	no flags	Details
Patch (17.89 KB, patch) 2018-06-26 20:40 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (17.89 KB, patch) 2018-06-26 21:21 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (18.29 KB, patch) 2018-06-27 09:10 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-cq-03 for mac-sierra (2.35 MB, application/zip) 2018-06-27 11:02 PDT, WebKit Commit Bot	no flags	Details
Patch (19.19 KB, patch) 2018-06-27 11:38 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (19.19 KB, patch) 2018-06-27 13:13 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-cq-01 for mac-sierra (2.36 MB, application/zip) 2018-06-27 14:21 PDT, WebKit Commit Bot	no flags	Details
Patch (22.12 KB, patch) 2018-06-27 14:33 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (2.06 KB, patch) 2018-06-28 10:56 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (3.36 KB, patch) 2018-06-28 11:09 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (4.29 KB, patch) 2018-06-28 11:55 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews103 for mac-sierra (2.41 MB, application/zip) 2018-06-28 13:03 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews115 for mac-sierra (3.44 MB, application/zip) 2018-06-28 13:59 PDT, EWS Watchlist	no flags	Details
Patch (5.31 KB, patch) 2018-06-28 14:08 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Show Obsolete (16) View All Add attachment proposed patch, testcase, etc.

Timothy Hatcher

Comment 1 2018-06-26 17:35:18 PDT Comment hidden (obsolete)

Created attachment 343664 [details] Patch

Timothy Hatcher

Comment 2 2018-06-26 18:33:22 PDT Comment hidden (obsolete)

Created attachment 343669 [details] Patch

Tim Horton

Comment 3 2018-06-26 19:09:52 PDT Comment hidden (obsolete)

Comment on attachment 343669 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343669&action=review > Source/WebCore/rendering/RenderTheme.h:268 > + // The platcform highlighting colors for TextMatches. Platc > Source/WebCore/rendering/RenderThemeMac.mm:397 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300 Probably should change TextIndicator too?

EWS Watchlist

Comment 4 2018-06-26 19:41:23 PDT Comment hidden (obsolete)

Comment on attachment 343669 [details] Patch Attachment 343669 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8355129 New failing tests: fast/text/mark-matches-rendering.html fast/text/mark-matches-broken-line-rendering.html

EWS Watchlist

Comment 5 2018-06-26 19:41:25 PDT Comment hidden (obsolete)

Created attachment 343680 [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

EWS Watchlist

Comment 6 2018-06-26 20:16:26 PDT Comment hidden (obsolete)

Comment on attachment 343669 [details] Patch Attachment 343669 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8355252 New failing tests: fast/text/mark-matches-rendering.html fast/text/mark-matches-broken-line-rendering.html

EWS Watchlist

Comment 7 2018-06-26 20:16:28 PDT Comment hidden (obsolete)

Created attachment 343681 [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

Timothy Hatcher

Comment 8 2018-06-26 20:40:14 PDT Comment hidden (obsolete)

Created attachment 343683 [details] Patch

Tim Horton

Comment 9 2018-06-26 21:04:35 PDT Comment hidden (obsolete)

Comment on attachment 343683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343683&action=review > Source/WebCore/rendering/RenderThemeMac.mm:406 > + // The inactive color is normaly used, since no legacy WebKit client marks a text match as active. s/normaly/normally/ Also, so weird! I can iterate the matches in e.g. the Xcode docs viewer, does that not use active state?

Timothy Hatcher

Comment 10 2018-06-26 21:16:39 PDT Comment hidden (obsolete)

Comment on attachment 343683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343683&action=review >> Source/WebCore/rendering/RenderThemeMac.mm:406 >> + // The inactive color is normaly used, since no legacy WebKit client marks a text match as active. > > s/normaly/normally/ > > Also, so weird! I can iterate the matches in e.g. the Xcode docs viewer, does that not use active state? Sorry for the typos. :| I was surprised too! That just changes the text selection, which overrides the text marker drawing.

Timothy Hatcher

Comment 11 2018-06-26 21:21:34 PDT Comment hidden (obsolete)

Created attachment 343685 [details] Patch

Timothy Hatcher

Comment 12 2018-06-26 21:37:33 PDT Comment hidden (obsolete)

Waiting on EWS.

WebKit Commit Bot

Comment 13 2018-06-27 08:54:16 PDT Comment hidden (obsolete)

Comment on attachment 343685 [details] Patch Rejecting attachment 343685 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: DEO_TRACK -DENABLE_VIDEO_USES_ELEMENT_FULLSCREEN -DENABLE_WEB_AUDIO -DENABLE_WEB_AUTHN -DENABLE_WEB_RTC -DENABLE_WEBGL -DENABLE_WEBGL2 -DENABLE_WEBGPU -DENABLE_WIRELESS_PLAYBACK_TARGET -DENABLE_XSLT -DBUILDING_WEBKIT -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -DGL_SILENCE_DEPRECATION=1 -DGLES_SILENCE_DEPRECATION=1 -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -fasm-blocks -fstrict-aliasing -Wprotocol -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.12 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/WebCore-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -IPAL -IForwardingHeaders -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/libxslt -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/libxml2 -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/WebKitAdditions -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/WebKitAdditions -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/webrtc -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc/sdk/objc/Framework/Headers -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/local/include/webrtc/sdk/objc/Framework/Headers -I/Volumes/Data/EWS/WebKit/Source/WebCore -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -Wno-unknown-warning-option -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/PrivateFrameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/System.framework/PrivateHeaders -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/Carbon.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/Quartz.framework/Frameworks -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/WebCorePrefix-aksclevtbjrwtmbxsnbvpaleabcg/WebCorePrefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource33-mm.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource33-mm.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource33-mm.mm -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource33-mm.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource21-mm.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource21-mm.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: https://webkit-queues.webkit.org/results/8358126

Timothy Hatcher

Comment 14 2018-06-27 09:10:19 PDT Comment hidden (obsolete)

Created attachment 343715 [details] Patch

WebKit Commit Bot

Comment 15 2018-06-27 11:02:46 PDT Comment hidden (obsolete)

Comment on attachment 343715 [details] Patch Rejecting attachment 343715 [details] from commit-queue. New failing tests: fast/text/mark-matches-rendering.html fast/text/mark-matches-broken-line-rendering.html Full output: https://webkit-queues.webkit.org/results/8359591

WebKit Commit Bot

Comment 16 2018-06-27 11:02:48 PDT Comment hidden (obsolete)

Created attachment 343727 [details] Archive of layout-test-results from webkit-cq-03 for mac-sierra The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-03 Port: mac-sierra Platform: Mac OS X 10.12.6

Timothy Hatcher

Comment 17 2018-06-27 11:38:46 PDT Comment hidden (obsolete)

Created attachment 343733 [details] Patch

Timothy Hatcher

Comment 18 2018-06-27 13:13:56 PDT Comment hidden (obsolete)

Created attachment 343744 [details] Patch

WebKit Commit Bot

Comment 19 2018-06-27 14:21:32 PDT Comment hidden (obsolete)

Comment on attachment 343744 [details] Patch Rejecting attachment 343744 [details] from commit-queue. New failing tests: fast/css/apple-system-control-colors.html fast/text/mark-matches-rendering.html fast/text/mark-matches-broken-line-rendering.html Full output: https://webkit-queues.webkit.org/results/8362362

WebKit Commit Bot

Comment 20 2018-06-27 14:21:34 PDT Comment hidden (obsolete)

Created attachment 343751 [details] Archive of layout-test-results from webkit-cq-01 for mac-sierra The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-01 Port: mac-sierra Platform: Mac OS X 10.12.6

Timothy Hatcher

Comment 21 2018-06-27 14:33:26 PDT

Created attachment 343753 [details] Patch

WebKit Commit Bot

Comment 22 2018-06-27 15:14:17 PDT Comment hidden (obsolete)

The commit-queue encountered the following flaky tests while processing attachment 343753 [details]: inspector/model/gradient.html bug 187118 (author: webkit@devinrousso.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 23 2018-06-27 15:15:01 PDT

Comment on attachment 343753 [details] Patch Clearing flags on attachment: 343753 Committed r233280: <https://trac.webkit.org/changeset/233280>

WebKit Commit Bot

Comment 24 2018-06-27 15:15:03 PDT Comment hidden (obsolete)

All reviewed patches have been landed. Closing bug.

Truitt Savell

Comment 25 2018-06-28 08:24:50 PDT Comment hidden (obsolete)

It looks like Revision 233280 introduced three consistent failures: fast/css/apple-system-control-colors.html fast/text/mark-matches-broken-line-rendering.html fast/text/mark-matches-rendering.html test results: https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r233302%20(5309)/results.html

Ryan Haddad

Comment 26 2018-06-28 09:23:00 PDT Comment hidden (obsolete)

The diffs show that the tests are expecting a more orange highlight on the text, but it the actual result is still yellow. I'm going to roll this out.

Tim Horton

Comment 27 2018-06-28 09:24:58 PDT

On 10.12 it *is* still yellow, and just needs a rebaseline there, probably?

Ryan Haddad

Comment 28 2018-06-28 09:29:04 PDT

(In reply to Tim Horton from comment #27) > On 10.12 it *is* still yellow, and just needs a rebaseline there, probably? It is also still yellow on 10.13 and 10.14 when dark mode isn't enabled, right?

Tim Horton

Comment 29 2018-06-28 09:35:13 PDT

No, now it uses findHighlightColor, which is likely not precisely the same color.

Timothy Hatcher

Comment 30 2018-06-28 10:47:11 PDT Comment hidden (obsolete)

I will land a new baseline for High Sierra.

Timothy Hatcher

Comment 31 2018-06-28 10:53:35 PDT

(In reply to Ryan Haddad from comment #28) > (In reply to Tim Horton from comment #27) > > On 10.12 it *is* still yellow, and just needs a rebaseline there, probably? > It is also still yellow on 10.13 and 10.14 when dark mode isn't enabled, > right? Yes, it is the same color in light and dark modes.

Timothy Hatcher

Comment 32 2018-06-28 10:54:10 PDT

Reopening to update layout tests.

Timothy Hatcher

Comment 33 2018-06-28 10:56:24 PDT Comment hidden (obsolete)

Created attachment 343817 [details] Patch

Timothy Hatcher

Comment 34 2018-06-28 11:09:16 PDT Comment hidden (obsolete)

Created attachment 343818 [details] Patch

Timothy Hatcher

Comment 35 2018-06-28 11:55:37 PDT Comment hidden (obsolete)

Created attachment 343828 [details] Patch

EWS Watchlist

Comment 36 2018-06-28 13:03:51 PDT Comment hidden (obsolete)

Comment on attachment 343828 [details] Patch Attachment 343828 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8372598 New failing tests: fast/css/apple-system-control-colors.html

EWS Watchlist

Comment 37 2018-06-28 13:03:53 PDT Comment hidden (obsolete)

Created attachment 343837 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 38 2018-06-28 13:59:53 PDT Comment hidden (obsolete)

Comment on attachment 343828 [details] Patch Attachment 343828 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8372851 New failing tests: fast/css/apple-system-control-colors.html

EWS Watchlist

Comment 39 2018-06-28 13:59:55 PDT Comment hidden (obsolete)

Created attachment 343845 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6

Timothy Hatcher

Comment 40 2018-06-28 14:08:12 PDT

Created attachment 343848 [details] Patch

WebKit Commit Bot

Comment 41 2018-06-28 14:48:52 PDT

Comment on attachment 343848 [details] Patch Clearing flags on attachment: 343848 Committed r233329: <https://trac.webkit.org/changeset/233329>

WebKit Commit Bot

Comment 42 2018-06-28 14:48:54 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.