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

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Timothy Hatcher]

White text on a yellow background is hard to read.

<rdar://problem/40354841>

[Timothy Hatcher]

Created attachment 343664 [details]
Patch

[Timothy Hatcher]

Created attachment 343669 [details]
Patch

[Tim Horton]

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 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]

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 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]

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]

Created attachment 343683 [details]
Patch

[Tim Horton]

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 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]

Created attachment 343685 [details]
Patch

[Timothy Hatcher]

Waiting on EWS.

[WebKit Commit Bot]

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]

Created attachment 343715 [details]
Patch

[WebKit Commit Bot]

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]

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]

Created attachment 343733 [details]
Patch

[Timothy Hatcher]

Created attachment 343744 [details]
Patch

[WebKit Commit Bot]

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]

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]

Created attachment 343753 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 343753 [details]:

inspector/model/gradient.html bug 187118 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 343753 [details]
Patch

Clearing flags on attachment: 343753

Committed r233280: <https://trac.webkit.org/changeset/233280>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

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]

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]

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

[Ryan Haddad]

(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]

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

[Timothy Hatcher]

I will land a new baseline for High Sierra.

[Timothy Hatcher]

(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]

Reopening to update layout tests.

[Timothy Hatcher]

Created attachment 343817 [details]
Patch

[Timothy Hatcher]

Created attachment 343818 [details]
Patch

[Timothy Hatcher]

Created attachment 343828 [details]
Patch

[EWS Watchlist]

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]

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 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]

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]

Created attachment 343848 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 343848 [details]
Patch

Clearing flags on attachment: 343848

Committed r233329: <https://trac.webkit.org/changeset/233329>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.