Bug 178331 – Web Inspector: [PARITY] Styles Redesign: Make filtering work

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

[Nikita Vasilyev]

Filtering at the bottom of the new styles sidebar should work similarly to the old styles sidebar.

[Radar WebKit Bug Importer]

<rdar://problem/35001015>

[Devin Rousso]

Created attachment 328284 [details]
Patch

[EWS Watchlist]

Comment on attachment 328284 [details]
Patch

Attachment 328284 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5473784

New failing tests:
http/tests/security/import-script-crossorigin-loads-omit.html

[EWS Watchlist]

Created attachment 328287 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Devin Rousso]

Comment on attachment 328287 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

Unrelated test failure.

[Timothy Hatcher]

Comment on attachment 328284 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:186
> +        if (!this.didInitialLayout)

Does filtering get applied automatically when layout does eventually happen?

[Devin Rousso]

Comment on attachment 328284 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:186
>> +        if (!this.didInitialLayout)
> 
> Does filtering get applied automatically when layout does eventually happen?

Yup!  `_renderSelector` gets called by `layout`.

[WebKit Commit Bot]

Comment on attachment 328284 [details]
Patch

Clearing flags on attachment: 328284

Committed r225569: <https://trac.webkit.org/changeset/225569>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Nikita Vasilyev]

Comment on attachment 328284 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:12
> +        Since both the sections and editors (per-section) use View semantics, we cannot simply
> +        search for instances of the filtered text since not all of the subviews may have called
> +        layout() yet. Instead, we have to rely on event listeners to relay information as to whether
> +        the filtered text was matched up the chain, applying the correct style classes along the way.

Unfortunately, asynchronous nature of WI.View makes things more complicated than they should be.

In some cases, batching DOM operations by WI.View can make things faster. In the styles sidebar, frequent DOM updates only happen when editing properties and they don't use WI.View anyway.

WI.View has also introduced Bug 179291 - Web Inspector: Styles Redesign: flashing when switching between nodes.

I'd rather not remove WI.View from the styles sidebar entirely, since it provides `detached` method that is used in WI.SpreadsheetCSSStyleDeclarationEditor. However, it would be nice to have a synchronous mode for WI.VIew.

[Blaze Burg]

Comment on attachment 328284 [details]
Patch

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

>> Source/WebInspectorUI/ChangeLog:12
>> +        the filtered text was matched up the chain, applying the correct style classes along the way.
> 
> Unfortunately, asynchronous nature of WI.View makes things more complicated than they should be.
> 
> In some cases, batching DOM operations by WI.View can make things faster. In the styles sidebar, frequent DOM updates only happen when editing properties and they don't use WI.View anyway.
> 
> WI.View has also introduced Bug 179291 - Web Inspector: Styles Redesign: flashing when switching between nodes.
> 
> I'd rather not remove WI.View from the styles sidebar entirely, since it provides `detached` method that is used in WI.SpreadsheetCSSStyleDeclarationEditor. However, it would be nice to have a synchronous mode for WI.VIew.

Is there some reason we cannot filter based on the model data rather than screwing around with the DOM views we created?