RESOLVED FIXED 195104
Web Inspector: Sources: disabled breakpoints banner should be sticky
https://bugs.webkit.org/show_bug.cgi?id=195104
Summary Web Inspector: Sources: disabled breakpoints banner should be sticky
Devin Rousso
Reported 2019-02-27 10:10:06 PST
Created attachment 363102 [details] [Image] Screenshot of Issue It's not very helpful if you have a lot of breakpoints and want to disable/enable them quickly.
Attachments
[Image] Screenshot of Issue (24.76 KB, image/png)
2019-02-27 10:10 PST, Devin Rousso
no flags
Patch (14.02 KB, patch)
2019-03-10 21:11 PDT, Devin Rousso
no flags
Patch (15.50 KB, patch)
2019-03-19 17:58 PDT, Devin Rousso
no flags
Patch (15.05 KB, patch)
2019-03-19 18:02 PDT, Devin Rousso
no flags
Patch (14.22 KB, patch)
2019-03-19 18:09 PDT, Devin Rousso
no flags
Patch (14.24 KB, patch)
2019-03-20 11:37 PDT, Devin Rousso
no flags
Radar WebKit Bug Importer
Comment 1 2019-02-27 10:51:13 PST
Devin Rousso
Comment 2 2019-03-10 21:11:49 PDT
Joseph Pecoraro
Comment 3 2019-03-14 00:09:11 PDT
Comment on attachment 364229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364229&action=review > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css:125 > +@media (min-height: 600px) { This is not described in the ChangeLog. What are you changing based on height? > Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1452 > + _handleCallStackElementAddedOrRemoved(event) > + { > + function walk(parent) { > + let count = 0; > + if (!parent.root) > + ++count; > + > + for (let child of parent.children) > + count += walk(child); > + > + // Don't count the main thread element when it's hidden. > + if (parent instanceof WI.ThreadTreeElement && WI.targets.length === 1) > + --count; > + > + return count; > + } > + this.element.style.setProperty("--call-stack-count", walk(this._callStackTreeOutline)); > + } How often does this happen? If huge call stack happens, such as (500 or 1000 elements is not uncommon on rAF loops), will this have been called 500 to walk the ...497,498,499,500 children? It might be nice to take advantage of the fact that we should know tree outline has a depth of 2, so maybe we could compute faster: let count = 0; count += this._callStackTreeOutline.children.length; for (let threadElement of this._callStackTreeOutline) threadElement += this._callStackTreeOutline.children.length; // Don't count the main thread element when it is hidden. if (WI.targets.length === 1) --count; Or maybe just enumerating the call frames list and not the tree elements.
Devin Rousso
Comment 4 2019-03-14 00:39:40 PDT
Comment on attachment 364229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364229&action=review >> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css:125 >> +@media (min-height: 600px) { > > This is not described in the ChangeLog. What are you changing based on height? I swear I wrote this down somewhere -.- Oh well :( The reason for this is at small heights, if we still were to disallow overflowing we'd end up in situations where each section would only be ~50px tall, which is barely enough to show more than one `WI.TreeElement` with the header of a `WI.DetailsSection`. I chose 600px because it seemed like a nice number where each section would get an even amount of space, assuming they were all each individually overflowing into their own scrollable areas (e.g. the `.pause-reason-container` wouldn't go over ~80px since it's a single element with a known description length, each of `.call-stack-container` and `.breakpoints-container` are limited to a max of `135px`, leaving a good amount of space for the resources list since the 600px also includes the toolbar/tabbar/prompt/etc). >> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1452 >> + } > > How often does this happen? If huge call stack happens, such as (500 or 1000 elements is not uncommon on rAF loops), will this have been called 500 to walk the ...497,498,499,500 children? > > It might be nice to take advantage of the fact that we should know tree outline has a depth of 2, so maybe we could compute faster: > > let count = 0; > count += this._callStackTreeOutline.children.length; > for (let threadElement of this._callStackTreeOutline) > threadElement += this._callStackTreeOutline.children.length; > > // Don't count the main thread element when it is hidden. > if (WI.targets.length === 1) > --count; > > Or maybe just enumerating the call frames list and not the tree elements. I'll rework this (and the one for breakpoints) to add `.children.length` to hopefully make it a bit quicker. Neither of them should ever have a depth more than 2, so I think that that would be fine.
Devin Rousso
Comment 5 2019-03-19 17:58:06 PDT
Devin Rousso
Comment 6 2019-03-19 18:02:47 PDT
Devin Rousso
Comment 7 2019-03-19 18:09:10 PDT
WebKit Commit Bot
Comment 8 2019-03-20 09:21:52 PDT Comment hidden (obsolete)
Devin Rousso
Comment 9 2019-03-20 11:37:12 PDT
WebKit Commit Bot
Comment 10 2019-03-20 12:37:22 PDT
Comment on attachment 365367 [details] Patch Clearing flags on attachment: 365367 Committed r243225: <https://trac.webkit.org/changeset/243225>
WebKit Commit Bot
Comment 11 2019-03-20 12:37:24 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.