Bug 197276

Summary: REGRESSION(r244635): [GTK] Wrong background color used in non-dark mode
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, ews-watchlist, mcatanzaro, timothy
Priority: P2 Keywords: Gtk, LayoutTestFailure, Regression
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=126907
https://bugs.webkit.org/show_bug.cgi?id=197947
Attachments:
Description Flags
Patch
mcatanzaro: review+
Patch
none
Patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-wk2
none
Patch for landing none

Carlos Garcia Campos
Reported 2019-04-25 02:49:53 PDT
Since r244635, we are now getting the frame view background color from the theme. That's correct for dark mode, but in non-dark mode we still want to use white backgrounds by default. This made a lot of tests to fail.
Attachments
Patch (3.07 KB, patch)
2019-04-25 02:54 PDT, Carlos Garcia Campos 		mcatanzaro: review+
DetailsFormatted DiffDiff
Patch (6.07 KB, patch)
2019-04-25 06:36 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch (6.14 KB, patch)
2019-04-26 00:56 PDT, Carlos Garcia Campos 		ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.53 MB, application/zip)
2019-04-26 03:00 PDT, EWS Watchlist 		no flags
Details
Patch for landing (6.03 KB, patch)
2019-04-29 00:57 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2019-04-25 02:54:16 PDT
Created attachment 368224 [details] Patch
Michael Catanzaro
Comment 2 2019-04-25 06:15:29 PDT
Comment on attachment 368224 [details] Patch Makes sense.
Carlos Garcia Campos
Comment 3 2019-04-25 06:31:22 PDT
The patch is not enough for other test failures, I'll submit an updated one soon. We will need to rebaseline others because of the change in the disabled text for text fields.
Carlos Garcia Campos
Comment 4 2019-04-25 06:36:27 PDT
Created attachment 368233 [details] Patch
Michael Catanzaro
Comment 5 2019-04-25 07:27:56 PDT
Comment on attachment 368233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368233&action=review > Source/WebCore/css/CSSValueKeywords.in:272 > +-webkit-text-control-background Can we think of a better name? -webkit-system-control-background? -webkit-text-system-background?
Carlos Garcia Campos
Comment 6 2019-04-25 23:05:26 PDT
(In reply to Michael Catanzaro from comment #5) > Comment on attachment 368233 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=368233&action=review > > > Source/WebCore/css/CSSValueKeywords.in:272 > > +-webkit-text-control-background > > Can we think of a better name? > > -webkit-system-control-background? This would match apple, I added text because this is only used for text fields and text areas, not all form controls have the same background color. > -webkit-text-system-background? This doesn't make any sense to me. I'll use -webkit-system-text-control-background then.
Carlos Garcia Campos
Comment 7 2019-04-26 00:56:37 PDT
Created attachment 368310 [details] Patch
EWS Watchlist
Comment 8 2019-04-26 03:00:49 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 9 2019-04-26 03:00:51 PDT Comment hidden (obsolete)
Carlos Garcia Campos
Comment 10 2019-04-26 06:01:37 PDT
(In reply to Build Bot from comment #8) > Comment on attachment 368310 [details] > Patch > > Attachment 368310 [details] did not pass ios-sim-ews (ios-simulator-wk2): > Output: https://webkit-queues.webkit.org/results/12003555 > > New failing tests: > fast/css/first-letter-anonymous-block-crash.html This was not caused by this patch.
Michael Catanzaro
Comment 11 2019-04-26 07:25:29 PDT
Maybe Timothy could propose a name; this becomes CSS API forever, so we want to make sure the name makes sense.
Timothy Hatcher
Comment 12 2019-04-26 10:33:53 PDT
(In reply to Michael Catanzaro from comment #11) > Maybe Timothy could propose a name; this becomes CSS API forever, so we want > to make sure the name makes sense. I would go with -webkit-text-background or -webkit-view-background. Since we have similar with -apple-system-text-background.
Michael Catanzaro
Comment 13 2019-04-26 11:14:19 PDT
This is equivalent to -apple-system-control-background, though.
Timothy Hatcher
Comment 14 2019-04-26 12:31:21 PDT
(In reply to Michael Catanzaro from comment #13) > This is equivalent to -apple-system-control-background, though. -apple-system is a prefix, separate from -apple and -webkit. So dropping "system" seems more fitting, and we don't have -webkit-system for any other keywords.
Michael Catanzaro
Comment 15 2019-04-26 15:20:15 PDT
I don't understand the difference between -apple-system and -apple, but OK. So would -webkit-control-background be appropriate, then?
Timothy Hatcher
Comment 16 2019-04-26 16:22:57 PDT
(In reply to Michael Catanzaro from comment #15) > I don't understand the difference between -apple-system and -apple, but OK. -apple is basically an alias for -webkit and -apple-system is separate and not mapped to -webkit. > So would -webkit-control-background be appropriate, then? That is fine.
Carlos Garcia Campos
Comment 17 2019-04-29 00:57:18 PDT
Created attachment 368445 [details] Patch for landing
Carlos Garcia Campos
Comment 18 2019-04-29 03:14:08 PDT
Committed r244731: <https://trac.webkit.org/changeset/244731>
Note You need to log in before you can comment on or make changes to this bug.