| Summary: | Web Inspector: [PARITY] Styles Redesign: Make filtering work | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bburg, commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, rniwa, timothy, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 180466 | ||||||||
| Attachments: |
|
||||||||
|
Description
Nikita Vasilyev
2017-10-15 20:55:49 PDT
Created attachment 328284 [details]
Patch
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 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
Comment on attachment 328287 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
Unrelated test failure.
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? 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`. Comment on attachment 328284 [details] Patch Clearing flags on attachment: 328284 Committed r225569: <https://trac.webkit.org/changeset/225569> All reviewed patches have been landed. Closing bug. 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. 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? |