Bug 234570

Summary: Web Inspector: Assertion Failed adding event listener in CSSPropertyNameCompletions.prototype._updateValuesWithLatestCSSVariablesIfNeeded
Product: WebKit Reporter: Patrick Angle <pangle>
Component: Web InspectorAssignee: Patrick Angle <pangle>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1.0 none

Description Patrick Angle 2021-12-21 11:01:41 PST
From Devin:
[Error] Assertion Failed (4)
DOMNodeStyles
"dom-node-styles-needs-refresh"
function _handleNodesStylesNeedsRefresh(event) { this._needsVariablesFromInspectedNode = true; }
CSSPropertyNameCompletions
	addEventListener (Object.js:53)
	_updateValuesWithLatestCSSVariablesIfNeeded (CSSPropertyNameCompletions.js:90)
	executeQuery (CSSPropertyNameCompletions.js:59)
	(anonymous function) (CSSKeywordCompletions.js:48)
	_nameCompletionDataProvider
	_updateCompletions (SpreadsheetTextField.js:436)
	_handleInput (SpreadsheetTextField.js:423)
	_handleInput

Currently we continually add event listeners every time we update the completions. We should only add/remove this listener on a node changing.
Comment 1 Radar WebKit Bug Importer 2021-12-21 11:01:52 PST
<rdar://problem/86776737>
Comment 2 Patrick Angle 2021-12-21 11:07:08 PST
Created attachment 447736 [details]
Patch v1.0
Comment 3 Devin Rousso 2021-12-21 11:12:18 PST
Comment on attachment 447736 [details]
Patch v1.0

r=me, this is perhaps maybe slightly more inefficient cause we're now listening for `WI.DOMNodeStyles.Event.NeedsRefresh` even if we haven't called `allCSSVariables` yet, but I think it's fine
Comment 4 EWS 2021-12-21 17:40:40 PST
Committed r287338 (245480@main): <https://commits.webkit.org/245480@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447736 [details].