[webkit-reviews] review granted: [Bug 186472] Link drag image is inconsistently unreadable in dark mode : [Attachment 342382] Patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has granted  review:
Bug 186472: Link drag image is inconsistently unreadable in dark mode
https://bugs.webkit.org/show_bug.cgi?id=186472

Attachment 342382: Patch

https://bugs.webkit.org/attachment.cgi?id=342382&action=review




--- Comment #6 from Timothy Hatcher <timothy at apple.com> ---
Comment on attachment 342382
  --> https://bugs.webkit.org/attachment.cgi?id=342382
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342382&action=review

>> Source/WebCore/platform/mac/DragImageMac.mm:306
>> +	LocalDefaultSystemAppearance localAppearance(true, page ?
page->defaultAppearance() : true);
> 
> We need a better way to allow getting the true effective appearance for
places like this that need it. This does not feel right as-is, considering the
changes to defaultAppearance().

I think this is fine now. Sorry for the confusion. I think I want to make a
patch that flips defaultAppearance to be prefersDarkInterface, so it is easier
to read everywhere.

>>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:5019
>>>	 NSAppearanceName appearance = [[m_view effectiveAppearance]
bestMatchFromAppearancesWithNames:@[ NSAppearanceNameAqua,
NSAppearanceNameDarkAqua ]];
>> 
>> Are we sure that we can do this without leaking dark mode to parts where we
are sure we *don’t* want it to be used?
> 
> Megan is right, we cannot do this here. It will cause Safari to use dark mode
colors and form controls for pages.

Actually, I think this is fine now! I forgot LocalDefaultSystemAppearance now
checks for both useSystemAppearance and useDefaultAppearance. In the Safari
case, useSystemAppearance will still be false.