<rdar://31137058>
Created attachment 433085 [details] Patch v1.0
Comment on attachment 433085 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=433085&action=review r=me > Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js:134 > + if (parentNode.attached && !parentNode.parentNode) { I think the only way for this to be true is if `parentNode` is the document, so it might be better to directly check for that instead (especially since we probably will eventually move to a future where nodes can be sent to the frontend without including parent info, so `attached` might be `true` even if the `parentNode` is `null`). That way you could also rename it to `documentNeedsUpdate`, which IMO is more understandable/reasonable as a reason to update the entire `WI.DOMTreeOutline`. > Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js:151 > + this._recentlyModifiedNodes.clear(); > + this._recentlyModifiedAttributes.clear(); NIT: Instead of duplicating these `clear()` here, you could take the entire `for` below and put it inside an `else` for this. I personally try to avoid early returns like this cause it means there's more state one has to keep track of when creating branches in the future, whereas if there's no `return` then everything always flows down the same path to the same end. > Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js:167 > - return; > + continue; LOL oops wonder how many other problems this caused 😅
Comment on attachment 433085 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=433085&action=review >> Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js:167 >> + continue; > > LOL oops wonder how many other problems this caused 😅 Such a good bug, I later fixed it anyways apparently https://bugs.webkit.org/show_bug.cgi?id=230289
Pull request: https://github.com/WebKit/WebKit/pull/3914
Committed 254062@main (f6c535899310): <https://commits.webkit.org/254062@main> Reviewed commits have been landed. Closing PR #3914 and removing active labels.