White text on a yellow background is hard to read. <rdar://problem/40354841>
Created attachment 343664 [details] Patch
Created attachment 343669 [details] Patch
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?
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
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
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
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
Created attachment 343683 [details] Patch
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?
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.
Created attachment 343685 [details] Patch
Waiting on EWS.
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
Created attachment 343715 [details] Patch
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
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
Created attachment 343733 [details] Patch
Created attachment 343744 [details] Patch
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
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
Created attachment 343753 [details] Patch
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.
Comment on attachment 343753 [details] Patch Clearing flags on attachment: 343753 Committed r233280: <https://trac.webkit.org/changeset/233280>
All reviewed patches have been landed. Closing bug.
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
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.
On 10.12 it *is* still yellow, and just needs a rebaseline there, probably?
(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?
No, now it uses findHighlightColor, which is likely not precisely the same color.
I will land a new baseline for High Sierra.
(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.
Reopening to update layout tests.
Created attachment 343817 [details] Patch
Created attachment 343818 [details] Patch
Created attachment 343828 [details] Patch
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
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
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
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
Created attachment 343848 [details] Patch
Comment on attachment 343848 [details] Patch Clearing flags on attachment: 343848 Committed r233329: <https://trac.webkit.org/changeset/233329>