[webkit-reviews] review granted: [Bug 234615] Web Inspector: Sources: cannot copy grouping of blackboxed call frames : [Attachment 448357] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 14 09:27:37 PST 2022

Patrick Angle <pangle at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 234615: Web Inspector: Sources: cannot copy grouping of blackboxed call

Attachment 448357: Patch


--- Comment #3 from Patrick Angle <pangle at apple.com> ---
Comment on attachment 448357
  --> https://bugs.webkit.org/attachment.cgi?id=448357

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


> The only downside I can think of as a result of this is that up/down will now
> through blackboxed call frame groups, meaning that it's an extra step (or
more) to
> select the previous/next call frame using the arrow keys when there's a
> call frame group in the middle.

I tried this patch locally, and this doesn't appear to be a problem. I can
arrow through the list of call frames as I would expect to be able to do, where
each press of the down key moves me down to the next visible call frame, even
if there are multiple blackboxes call frames represented by the currently
selected frame.

> Source/WebInspectorUI/UserInterface/Views/TreeElement.js:316
> +	   if (treeElement.toggleOnClick ||
treeElement.isEventWithinDisclosureTriangle(event)) {

Aside: It's kinda strange to me that several subclasses of TreeElement have a
toggleOnClick member set in their constructor, but TreeElement itself
doesn't... I wonder if it makes more sense for there to be a public `get
toggleOnClick()` in TreeElement that returns `false`, and is override by
subclasses that want to return `true`.

More information about the webkit-reviews mailing list