Summary: | [macOS] Date pickers should respect the document's color scheme | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aditya Keerthi <akeerthi> | ||||||||
Component: | Forms | Assignee: | Aditya Keerthi <akeerthi> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, mifenton, thorton, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari Technology Preview | ||||||||||
Hardware: | Mac | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Aditya Keerthi
2020-09-14 17:41:04 PDT
Created attachment 408774 [details]
Patch
Comment on attachment 408774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408774&action=review > Source/WebCore/html/HTMLInputElement.cpp:2140 > + if (auto* computedStyle = this->computedStyle()) { I assume this null check is not actually necessary, since the old code did not. Is that true? > Source/WebKit/UIProcess/mac/WebDateTimePickerMac.mm:221 > + [_enclosingWindow setAppearance:[NSAppearance appearanceNamed:_params.useDarkAppearance ? NSAppearanceNameDarkAqua : NSAppearanceNameAqua]]; This is interesting! I feel like content that pops out of the page (like, say, context menus) usually follows the app, not the page. But probably this makes sense? (In reply to Tim Horton from comment #3) > Comment on attachment 408774 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408774&action=review > > > Source/WebCore/html/HTMLInputElement.cpp:2140 > > + if (auto* computedStyle = this->computedStyle()) { > > I assume this null check is not actually necessary, since the old code did > not. Is that true? The check is not actually necessary, since this method is only called when the element is in the node tree. Should I replace the check with an ASSERT? Comment on attachment 408774 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408774&action=review >> Source/WebKit/UIProcess/mac/WebDateTimePickerMac.mm:221 >> + [_enclosingWindow setAppearance:[NSAppearance appearanceNamed:_params.useDarkAppearance ? NSAppearanceNameDarkAqua : NSAppearanceNameAqua]]; > > This is interesting! I feel like content that pops out of the page (like, say, context menus) usually follows the app, not the page. But probably this makes sense? You can just leave it implicit, too. No reason to ASSERT() something that we expect will never happen and which will result in a crash in release, IMO. Created attachment 408872 [details]
Patch for landing
ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!. Created attachment 408889 [details]
Patch for landing.
Committed r267131: <https://trac.webkit.org/changeset/267131> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408889 [details]. |