[webkit-reviews] review denied: [Bug 61468] Web Inspector: Timeline panel improvements for managing current selection : [Attachment 95257] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 28 09:03:03 PDT 2011


Pavel Feldman <pfeldman at chromium.org> has denied tonistiigi at gmail.com's request
for review:
Bug 61468: Web Inspector: Timeline panel improvements for managing current
selection
https://bugs.webkit.org/show_bug.cgi?id=61468

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

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

A couple of tiny nits and a transition question. Otherwise looks good.

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:343
> +	   if(event.wheelDeltaX != 0){

if (event.wheelDeltaX !== 0) {
}

(missing spaces and != -> !==

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:372
> +    ResizerOffset: 3.5, // half pixel because offset values are not rounded
but ceiled

We don't define constants on prototypes, typically they are just
WebInspector.TimelineOverviewPane.ResizerOffset.

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:374
> +    WindowScrollSpeedFactor: .3

ditto

> Source/WebCore/inspector/front-end/inspector.css:2965
> +    -webkit-transition: top 0.3s ease-out 0.2s;

Is this one smoothing labels flickering?  When I implemented it originally,
labels were not flickering at all (see Safari 5 and earlier Chrome builds). It
just regressed at some point. I'd rather find out why.


More information about the webkit-reviews mailing list