Bug 192124

Summary:

Web Inspector: Styles: selection lost when inspector is blurred

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Nikita Vasilyev <nvasilyev>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, hi, inspector-bugzilla-changes, nvasilyev, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
[Video] With patch applied	none

Description Devin Rousso 2018-11-28 16:16:12 PST

* STEPS TO REPRODUCE:
1. inspect any page
2. show the Elements tab
3. select multiple CSS properties within the same CSS rule
4. switch WebKit/Safari tabs
5. go back to the original WebKit/Safari tab
 => selected properties no longer selected

Comment 1 Radar WebKit Bug Importer 2018-12-17 21:34:01 PST

<rdar://problem/46800965>

Comment 2 Nikita Vasilyev 2019-01-02 18:00:53 PST

Created attachment 358233 [details]
Patch

Comment 3 Nikita Vasilyev 2019-01-02 18:05:50 PST

Created attachment 358235 [details]
[Video] With patch applied

Comment 4 Devin Rousso 2019-01-02 21:28:58 PST

Comment on attachment 358233 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:137
> +    background-color: var(--selected-background-color-unfocused);

Aside: is there a reason we didn't make it so that `--background-color-selected` changes it's value when `.window-docked-inactive` or `.window-inactive` is set?  We use this in a few places, usually when we want to limit `:focus` to a particular element (not the <body>), but I'd imagine that it would apply just as well when set from these classes too.

Comment 5 Nikita Vasilyev 2019-01-02 22:52:49 PST

Comment on attachment 358233 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:137
>> +    background-color: var(--selected-background-color-unfocused);
> 
> Aside: is there a reason we didn't make it so that `--background-color-selected` changes it's value when `.window-docked-inactive` or `.window-inactive` is set?  We use this in a few places, usually when we want to limit `:focus` to a particular element (not the <body>), but I'd imagine that it would apply just as well when set from these classes too.

I find this easier to follow than:

    body:matches(.window-docked-inactive, .window-inactive) .spreadsheet-style-declaration-editor {
        --background-color-selected: var(--selected-background-color-unfocused);
    }

The latter adds an extra level of abstraction which I don't find necessary.

Comment 6 WebKit Commit Bot 2019-01-02 23:18:40 PST

Comment on attachment 358233 [details]
Patch

Clearing flags on attachment: 358233

Committed r239588: <https://trac.webkit.org/changeset/239588>

Comment 7 WebKit Commit Bot 2019-01-02 23:18:41 PST

All reviewed patches have been landed.  Closing bug.

Comment 8 Devin Rousso 2019-01-03 09:22:11 PST

Comment on attachment 358233 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:137
>>> +    background-color: var(--selected-background-color-unfocused);
>> 
>> Aside: is there a reason we didn't make it so that `--background-color-selected` changes it's value when `.window-docked-inactive` or `.window-inactive` is set?  We use this in a few places, usually when we want to limit `:focus` to a particular element (not the <body>), but I'd imagine that it would apply just as well when set from these classes too.
> 
> I find this easier to follow than:
> 
>     body:matches(.window-docked-inactive, .window-inactive) .spreadsheet-style-declaration-editor {
>         --background-color-selected: var(--selected-background-color-unfocused);
>     }
> 
> The latter adds an extra level of abstraction which I don't find necessary.

I'd actually meant doing something like this in Variables.css, considering we already have `body.window-inactive` and `body.window-inactive *` rules.