Bug 198508

Summary: REGRESSION(r244268): Web Inspector: Styles: navigating from Computed to Styles doesn't work
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Nikita Vasilyev
Reported 2019-06-03 17:22:20 PDT
Steps: 1. Open https://webkit.org 2. Inspect <body> 3. Open Computed panel in the right sidebar 4. Hover "background-color: rgb(0, 37, 61)" 5. Click on the arrow icon on the right. Expected: Styles panel open with "background-color" property selected. Actual: Styles panel open but no properties are selected. Notes: This must have regressed in the last 2 months.
Attachments
Patch (1.99 KB, patch)
2019-06-08 17:31 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Patch (3.56 KB, patch)
2019-06-10 13:07 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-06-03 17:22:38 PDT
<rdar://problem/51375503>
Nikita Vasilyev
Comment 2 2019-06-03 17:34:57 PDT
Broken in r244268 Web Inspector: sidebar panels shouldn't be added as subviews unless visible
Nikita Vasilyev
Comment 3 2019-06-03 17:41:54 PDT
After clicking the arrow icon: [Error] TypeError: null is not an object (evaluating 'this.parentSidebar.sidebarPanels') computedStyleDetailsPanelShowProperty (GeneralStyleDetailsSidebarPanel.js:75) spreadsheetCSSStyleDeclarationEditorShowProperty (ComputedStyleDetailsPanel.js:78) spreadsheetStylePropertyShowProperty (ComputedStyleSection.js:207) (anonymous function) (SpreadsheetStyleProperty.js:227)
Nikita Vasilyev
Comment 4 2019-06-08 17:31:58 PDT
Created attachment 371687 [details] Patch
Matt Baker
Comment 5 2019-06-10 12:37:26 PDT
Comment on attachment 371687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371687&action=review > Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.js:72 > + let parentSidebar = this.parentSidebar; Instead of saving a reference, why not reorder the operations in the function body? computedStyleDetailsPanelShowProperty(property) { let styleRulesPanel = this.parentSidebar.sidebarPanels.find((panel) => panel instanceof WI.RulesStyleDetailsSidebarPanel); console.assert(styleRulesPanel, "Styles panel is missing."); if (!styleRulesPanel) return; this.parentSidebar.selectedSidebarPanel = styleRulesPanel; styleRulesPanel.panel.scrollToSectionAndHighlightProperty(property); } Also, this delegate method seems like it belongs in ComputedStyleDetailsPanel, not in a generic base class.
Nikita Vasilyev
Comment 6 2019-06-10 13:00:55 PDT
(In reply to Matt Baker from comment #5) > Comment on attachment 371687 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=371687&action=review > > > Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.js:72 > > + let parentSidebar = this.parentSidebar; > > Instead of saving a reference, why not reorder the operations in the > function body? Makes sense. Also, I like how you didn't use a string identifier.
Nikita Vasilyev
Comment 7 2019-06-10 13:07:31 PDT
Created attachment 371772 [details] Patch
Matt Baker
Comment 8 2019-06-10 13:28:12 PDT
Comment on attachment 371772 [details] Patch r=me
WebKit Commit Bot
Comment 9 2019-06-10 14:14:01 PDT
Comment on attachment 371772 [details] Patch Clearing flags on attachment: 371772 Committed r246279: <https://trac.webkit.org/changeset/246279>
WebKit Commit Bot
Comment 10 2019-06-10 14:14:03 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.