Bug 189673 – Web Inspector: move DarkMode.css rules into appropriate CSS files

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Move all CSS rules from DarkMode.css to appropriate CSS files, then remove DarkMode.css.

DarkMode.css consists of many sections, e.g.:

    /* ImageResourceContentView.css */
    .content-view.resource.image {
        background: var(--background-color-content);
    }

This CSS rule, for example, should move to ImageResourceContentView.css.

<rdar://problem/43648004>

[Nikita Vasilyev]

Created attachment 349923 [details]
WIP

Well, this broke a few things since the order of the style rules changes. Priorities of some CSS rules need to be adjusted.

[Nikita Vasilyev]

Created attachment 349924 [details]
[Screenshot] WIP

[Nikita Vasilyev]

Created attachment 349927 [details]
WIP

[Nikita Vasilyev]

Created attachment 350152 [details]
Patch

I filed Bug 189766 - Web Inspector: Dark Mode: use the same CSS variables for dark and light modes,
which depends on this patch.

[Matt Baker]

Comment on attachment 350152 [details]
Patch

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

r=me, with a few minor things.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:336
> +    but close enough for now. It needs to use partial translucency so that the selection area shines through. */

Is there a bug that can be referenced here?

> Source/WebInspectorUI/UserInterface/Views/Main.css:136
> +    background-color: var(--background-color-content);

Can this change go in https://webkit.org/b/189766 instead?

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:82
> +@media all {

So the banner style applies to both light and dark mode? Why do we need "@media all" here?

> Source/WebInspectorUI/UserInterface/Views/TabBar.css:368
> +        /* FIXME: Use semantic colors */

Is there a bug that can be referenced here?

> Source/WebInspectorUI/UserInterface/Views/Variables.css:144
> +    /* FIXME: Use CSS4 color functions once they're available. */

Could you file a bug for this and add a reference?

> Source/WebInspectorUI/UserInterface/Views/Variables.css:266
> +        /* FIXME: these properties are duplicated so that they have higher specificity than WebKit's stylesheet.

If the workaround isn't temporary, you can drop the FIXME from this explanatory comment.

[Nikita Vasilyev]

Comment on attachment 350152 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.css:82
>> +@media all {
> 
> So the banner style applies to both light and dark mode? Why do we need "@media all" here?

Whoops! Should be prefers-dark-interface!

>> Source/WebInspectorUI/UserInterface/Views/Variables.css:144
>> +    /* FIXME: Use CSS4 color functions once they're available. */
> 
> Could you file a bug for this and add a reference?

AFAIK, there no syntax for this that vendors agreed on. I'll just remove this comment.

[Nikita Vasilyev]

Created attachment 350160 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 350160 [details]
Patch

Clearing flags on attachment: 350160

Committed r236237: <https://trac.webkit.org/changeset/236237>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Nikita Vasilyev]

(In reply to Matt Baker from comment #5)
> > Source/WebInspectorUI/UserInterface/Views/Main.css:136
> > +    background-color: var(--background-color-content);
> 
> Can this change go in https://webkit.org/b/189766 instead?

I re-added it in Bug 189852 - REGRESSION(r236237): Web Inspector: DarkMode: white background in Elements and Timelines 😬