Closed Bug 1144779 Opened 9 years ago Closed 9 years ago

Do not use mouse coordinates from onMouseUp to set selection end when dragging in Graph

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P2)

x86
macOS
defect

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

Right now the coordinates from the mouseup event are used to finalize the selection in the Graph.  This can cause weirdness when a mouseup happens outside of the toolbox and then it is triggered from the next mousemove.

STR:

Start a new selection on graph
Move the mouse up to the browser content
Release mouse
Move the mouse back into the timeline

Notice that the selection ends at the location of the mouse currently, when it should stay where it was when the mouse went up.

Dragging and resizing an existing selection doesn't have this behavior, because the coordinates are used from mousemove instead.
Attached patch timeline-mouseup.patch (obsolete) — Splinter Review
What do you think?
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment on attachment 8579478 [details] [diff] [review]
timeline-mouseup.patch

Forgot to flag r?
Attachment #8579478 - Flags: review?(vporof)
Blocks: 1144799
Comment on attachment 8579478 [details] [diff] [review]
timeline-mouseup.patch

Review of attachment 8579478 [details] [diff] [review]:
-----------------------------------------------------------------

Need to fix bug 1066504 first. Sorry!
Attachment #8579478 - Flags: review?(vporof)
Moving into the Profiler component. Filter on GUTHRIE'S WAVY CAKES.
Component: Developer Tools: Timeline → Developer Tools: Profiler
No longer depends on: 1066504
Brian, are you still planning on looking at these bugs? ( https://bugzilla.mozilla.org/showdependencytree.cgi?id=1147941&hide_resolved=1 )
Priority: -- → P2
(In reply to Victor Porof [:vporof][:vp] from comment #5)
> Brian, are you still planning on looking at these bugs? (
> https://bugzilla.mozilla.org/showdependencytree.
> cgi?id=1147941&hide_resolved=1 )

Yes, I'll take a look at this
I don't think this is as big of a deal anymore with Bug 1145784 fixed.  I'm not positive if there is still a case where the mouseup event will be skipped when bound to the top level window.  Some quick testing shows that the mouseup event fires even when I release it outside of the browser, but I'm not sure if that's dependent on the platform / window manager.  I think this will be a quick, easy fix so I'll proceed with a patch.
Depends on: 1145784
Don't use event coordinates from mouseup.  Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ee94157fbe4
Attachment #8579478 - Attachment is obsolete: true
Attachment #8602287 - Flags: review?(vporof)
Attachment #8602287 - Flags: review?(vporof) → review+
Also, thank you Brian for tackling those selection bugs!
Keywords: checkin-needed
remote:   https://hg.mozilla.org/integration/fx-team/rev/056ceb0ad5ea
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/056ceb0ad5ea
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 40
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.