WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 183015
Web Inspector: Canvas tab: ensure that the Recording TreeOutline has a specified height for virtualization
https://bugs.webkit.org/show_bug.cgi?id=183015
Summary
Web Inspector: Canvas tab: ensure that the Recording TreeOutline has a specif...
Devin Rousso
Reported
2018-02-21 13:20:06 PST
The Recording TreeOutline was added to its own container, but that container's height was never constrained, meaning that it's height is the full height of all the TreeElement children. Since TreeOutline uses `offsetHeight` to determine how many elements to show, the virtualization logic of TreeOutline then proceeds to add everything back to the DOM since it thinks it has room.
Attachments
Patch
(6.60 KB, patch)
2018-02-21 13:25 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(4.99 KB, patch)
2018-03-07 13:04 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Devin Rousso
Comment 1
2018-02-21 13:25:23 PST
Created
attachment 334409
[details]
Patch
Radar WebKit Bug Importer
Comment 2
2018-02-21 13:26:13 PST
<
rdar://problem/37758441
>
Matt Baker
Comment 3
2018-03-07 12:42:04 PST
Comment on
attachment 334409
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=334409&action=review
r-, because it looks like this breaks the recording tree outline: 1. Goto
https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Basic_animations
2. Record frames from "a looping panorama" demo 3. Open recording, expand first frame => No child elements
> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:716 > + if (!isNaN(this._virtualizedPreviousFirstItem) && Math.abs(firstItem - this._virtualizedPreviousFirstItem) < numberVisible)
Why is this needed? The bug title/summary gives the impression that the problem is with how TreeOutline is used in the CanvasSidebarPanel, not with TreeOutline itself. Please add a clarifying comment to the change log.
Devin Rousso
Comment 4
2018-03-07 13:04:12 PST
Comment on
attachment 334409
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=334409&action=review
>> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:716 >> + if (!isNaN(this._virtualizedPreviousFirstItem) && Math.abs(firstItem - this._virtualizedPreviousFirstItem) < numberVisible) > > Why is this needed? The bug title/summary gives the impression that the problem is with how TreeOutline is used in the CanvasSidebarPanel, not with TreeOutline itself. Please add a clarifying comment to the change log.
This is what caused the breakage. I was trying to optimize the virtualization functionality, but I see that it needs some tweaking. :P
Devin Rousso
Comment 5
2018-03-07 13:04:57 PST
Created
attachment 335208
[details]
Patch
Matt Baker
Comment 6
2018-03-07 14:39:19 PST
Comment on
attachment 335208
[details]
Patch r=me
WebKit Commit Bot
Comment 7
2018-03-07 15:09:05 PST
Comment on
attachment 335208
[details]
Patch Clearing flags on attachment: 335208 Committed
r229377
: <
https://trac.webkit.org/changeset/229377
>
WebKit Commit Bot
Comment 8
2018-03-07 15:09:07 PST
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.
Top of Page
Format For Printing
XML
Clone This Bug