[webkit-reviews] review denied: [Bug 101749] Web Inspector: Timeline: enhance short-records filter. : [Attachment 173300] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied eustas.bug at gmail.com's request
for review:
Bug 101749: Web Inspector: Timeline: enhance short-records filter.
https://bugs.webkit.org/show_bug.cgi?id=101749

Attachment 173300: Patch
https://bugs.webkit.org/attachment.cgi?id=173300&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173300&action=review


> Source/WebCore/ChangeLog:10
> +	   will make this feature much more useful.

What is this assessment based on?

> Source/WebCore/inspector/front-end/ProfilesPanel.js:1028
>	   this.profileViewStatusBarItemsContainer.style.left =
Math.max(minFloatingStatusBarItemsOffset, this.splitView.sidebarWidth()) +
"px";

This will result as a warning on your timeline, no? You are measuring
totalOffsetLeft and applying it to the style. Is that intended?

> Source/WebCore/inspector/front-end/StatusBarButton.js:32
> + * @typedef {{element: !Element, disabled: boolean}}

We don't use typedefs, ui components are compiled together.

> Source/WebCore/inspector/front-end/StatusBarButton.js:286
> +    set disabled(value)

Please do not create new getters / setters. They are not compiler friendly.