Add support for painting document markers using system colors #10601
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pxlcoder
added
the
New Bugs
pxlcoder
requested review from
whsieh,
megangardner,
rr-codes and
hortont424
|
EWS run on previous version of this PR (hash 4f0ade9)
|
whsieh
approved these changes
Feb 23, 2023
|
Thanks for the review! |
megangardner
approved these changes
Feb 23, 2023
| #endif // USE(APPLE_INTERNAL_SDK) | ||
|
|
||
| #if HAVE(NSSPELLCHECKER_CORRECTION_INDICATOR_UNDERLINE_COLOR) | ||
| @interface NSSpellChecker (Staging_105286196) |
There was a problem hiding this comment.
If you haven't, can you file a bug for removing this staging code?
There was a problem hiding this comment.
Filed a bug and added a comment.
| Color RenderThemeCocoa::platformDictationAlternativesMarkerColor(OptionSet<StyleColorOptions> options) const | ||
| { | ||
| auto useDarkMode = options.contains(StyleColorOptions::UseDarkAppearance); | ||
| return useDarkMode ? SRGBA<uint8_t> { 40, 145, 255, 217 } : SRGBA<uint8_t> { 0, 122, 255, 191 }; |
There was a problem hiding this comment.
Is there a way we can factor out all the same special numbers so that it's more clear that they're all the same thing? I'm also assuming there's no way to get these from the system or elsewhere and they need to be manually defined.
There was a problem hiding this comment.
I was doing some digging and it appears that there are system colors that we can use for these!
Though I will need to audit them individually to compare behavior. Since this hardcoding is not new I have filed https://bugs.webkit.org/show_bug.cgi?id=252868 to address this separately.
For now, I have updated the PR to unify the dictation alternative paths, and share the same color method, reducing the amount of duplication.
|
EWS run on current version of this PR (hash 1173f0e)
|
pxlcoder
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=252791 rdar://105796291 Reviewed by Wenson Hsieh and Megan Gardner. Use system colors for painting document markers when possible. To support this functionality, new `RenderTheme` methods are added to control platform-specific marker colors and the color cache is expanded for the new colors. This approach matches the implementation of existing system colors. * Source/WebCore/PAL/pal/spi/mac/NSSpellCheckerSPI.h: * Source/WebCore/rendering/RenderTheme.cpp: (WebCore::RenderTheme::spellingMarkerColor const): (WebCore::RenderTheme::platformSpellingMarkerColor const): (WebCore::RenderTheme::dictationAlternativesMarkerColor const): (WebCore::RenderTheme::platformDictationAlternativesMarkerColor const): (WebCore::RenderTheme::autocorrectionReplacementMarkerColor const): (WebCore::RenderTheme::platformAutocorrectionReplacementMarkerColor const): (WebCore::RenderTheme::grammarMarkerColor const): (WebCore::RenderTheme::platformGrammarMarkerColor const): (WebCore::RenderTheme::documentMarkerLineColor const): * Source/WebCore/rendering/RenderTheme.h: * Source/WebCore/rendering/RenderThemeCocoa.h: * Source/WebCore/rendering/RenderThemeCocoa.mm: (WebCore::RenderThemeCocoa::platformSpellingMarkerColor const): (WebCore::RenderThemeCocoa::platformDictationAlternativesMarkerColor const): (WebCore::RenderThemeCocoa::platformAutocorrectionReplacementMarkerColor const): (WebCore::RenderThemeCocoa::platformGrammarMarkerColor const): (WebCore::grammarColor): Deleted. (WebCore::RenderThemeCocoa::documentMarkerLineColor const): Deleted. * Source/WebCore/rendering/RenderThemeMac.h: * Source/WebCore/rendering/RenderThemeMac.mm: (WebCore::usePlatformColorForAutocorrectionReplacementMarker): (WebCore::RenderThemeMac::platformAutocorrectionReplacementMarkerColor const): Canonical link: https://commits.webkit.org/260779@main
webkit-early-warning-system
force-pushed
the
eng/252791
branch
from
1173f0e
to
68ae85e
Compare
|
Committed 260779@main (68ae85e): https://commits.webkit.org/260779@main Reviewed commits have been landed. Closing PR #10601 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
68ae85e
1173f0e
🛠 mac🛠 wpe🛠 wincairo🛠 mac-AS-debug🧪 ios-wk2🧪 api-mac🧪 gtk-wk2🧪 api-ios🧪 mac-wk1🧪 api-gtk🧪 mac-wk2🛠 tv-sim🧪 mac-AS-debug-wk2🛠 watch🧪 mac-wk2-stress