Bug 186577 - Input form controls have white background in dark mode when they should not
Summary: Input form controls have white background in dark mode when they should not
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-12 15:22 PDT by Timothy Hatcher
Modified: 2018-06-13 14:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (8.74 KB, patch)
2018-06-12 15:27 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (6.54 MB, application/zip)
2018-06-12 17:34 PDT, EWS Watchlist
no flags Details
Patch (12.85 KB, patch)
2018-06-13 10:06 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Patch (12.85 KB, patch)
2018-06-13 10:07 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2018-06-12 15:22:42 PDT
The input, textarea, and select have the wrong text and background color in dark mode.

<rdar://problem/39258325>
Comment 1 Timothy Hatcher 2018-06-12 15:27:14 PDT
Created attachment 342603 [details]
Patch
Comment 2 Megan Gardner 2018-06-12 15:33:19 PDT
Comment on attachment 342603 [details]
Patch

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

r=me with comments, unofficially

> Source/WebCore/ChangeLog:12
> +        (WebCore::featureWithValidIdent): Allow dark mode media query in user agenst stylesheets.

agenst?

> Source/WebCore/ChangeLog:17
> +        * css/html.css: Se color and background-color to semantic colors for input, textarea, and select.

Se?

> Source/WebCore/css/html.css:925
> +    /* While not clearest name, the inactiveborder system color maps to [NSColor controlBackgroundColor] on Mac. */

Should we just add an additional color with a more clear name rather than have all these comments?
Comment 3 Tim Horton 2018-06-12 15:39:11 PDT
Comment on attachment 342603 [details]
Patch

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

> Source/WebCore/css/html.css:389
> +    /* While not clearest name, the inactiveborder system color maps to [NSColor controlBackgroundColor] on Mac. */

Probably should add -apple-system-control-background or something
Comment 4 EWS Watchlist 2018-06-12 17:34:10 PDT
Comment on attachment 342603 [details]
Patch

Attachment 342603 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/8153561

Number of test failures exceeded the failure limit.
Comment 5 EWS Watchlist 2018-06-12 17:34:11 PDT
Created attachment 342613 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 6 Timothy Hatcher 2018-06-13 10:06:25 PDT
Created attachment 342670 [details]
Patch
Comment 7 Timothy Hatcher 2018-06-13 10:07:51 PDT
Created attachment 342671 [details]
Patch
Comment 8 Megan Gardner 2018-06-13 11:20:22 PDT
Comment on attachment 342671 [details]
Patch

r=me, unofficial
Comment 9 WebKit Commit Bot 2018-06-13 13:26:39 PDT
Comment on attachment 342671 [details]
Patch

Clearing flags on attachment: 342671

Committed r232806: <https://trac.webkit.org/changeset/232806>
Comment 10 WebKit Commit Bot 2018-06-13 13:26:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 mitz 2018-06-13 14:04:45 PDT
Comment on attachment 342671 [details]
Patch

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

> Source/WebCore/DerivedSources.make:1026
> +ifeq ($(shell $(CC) -std=gnu++17 -x c++ -E -P -dM $(SDK_FLAGS) $(TARGET_TRIPLE_FLAGS) $(FRAMEWORK_FLAGS) $(HEADER_FLAGS) -include "wtf/Platform.h" /dev/null | grep ' WTF_PLATFORM_MAC ' | cut -d' ' -f3), 1)

Not entirely new to this patch, but can we avoid invoking the C preprocessor (with the exact same command line!) once per macro whose value we need?