Bug 61468

Created attachment 94858 [details] Patch Also fixes: When selection is dragged with mouse slightly(1-2px) to the right, the selection first starts moving to the left(caused by the negative offset of the resizer).

Comment on attachment 94858 [details] Patch Patch looks good overall but there is a problem with double click handler: it will clear column labels in the overview pane(see screenshot). I'd try calling _setWindowPosition instead of .reset() r- for now.

Created attachment 94969 [details] Screenshot with cleared column labels

Created attachment 95257 [details] Patch Fixed the issue with double click on the labels.

Comment on attachment 95257 [details] Patch 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.

(In reply to comment #5) > > 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. > Strange because datagrid.js that I previously changed had variables with the same meaning done this way. To me it would make more sense to define it in prototype if its meaning is for object's internal use only(to save typing and lookup cost) and to use normal constants when it has some meaning for other objects. For example in timelineoverview, the resizer is private so why should any other object care about ResizerOffset constant. > > 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. It doesn't indeed happen in Safari5(but happens on Chrome stable). I'll look more into it later because it is definitely better this way than the workaround with transition. I though it isn't possible to keep it from moving without position:fixed but I guess I was wrong.

> Strange because datagrid.js that I previously changed had variables with the same meaning done this way. To me it would make more sense to define it in prototype if its meaning is for object's internal use only(to save typing and lookup cost) and to use normal constants when it has some meaning for other objects. > > For example in timelineoverview, the resizer is private so why should any other object care about ResizerOffset constant. > We would normally do either - WebInspector.TimelineOverviewPane._ResizerOffset or - this._resizerOffset initialized in the constructor for private values. I don't think lookup cost is significant in the first case, but I agree that typing is a bit painful. I do realize the benefits of your approach, but I think that consistency (where prototypes only have methods) is at least as important. I think DataGrid is more of an exception / oversight.

(In reply to comment #5) > > 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. I tracked it down to http://trac.webkit.org/changeset/75555/ that is non-inspector change that makes scrolling events fire async. So I guess my options are: - leave this part out of the patch - still use the transition for better but not ideal solution. - try to move labels outside the element with the overflow:auto. This will not be pretty as this element is outside of TimelineGrid. - any other?

(In reply to comment #8) > (In reply to comment #5) > > > > 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. > > I tracked it down to http://trac.webkit.org/changeset/75555/ that is non-inspector change that makes scrolling events fire async. > Makes sense, thanks. > So I guess my options are: > - leave this part out of the patch I'd do this. > - still use the transition for better but not ideal solution. > - try to move labels outside the element with the overflow:auto. This will not be pretty as this element is outside of TimelineGrid. Either this or we could try position: fixed and see if it actually slows down the scrolling. But it should probably be done in a separate patch anyways. > - any other?

Created attachment 95471 [details] Patch Removed transition. Fixed style issues.

The commit-queue encountered the following flaky tests while processing attachment 95471 [details]: http/tests/websocket/tests/frame-length-longer-than-buffer.html bug 61837 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 95471 [details] Patch Clearing flags on attachment: 95471 Committed r87776: <http://trac.webkit.org/changeset/87776>

All reviewed patches have been landed. Closing bug.