If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 40

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

unspecified
Firefox 40
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Created attachment 8579478 [details] [diff] [review]
timeline-mouseup.patch

What do you think?
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Comment on attachment 8579478 [details] [diff] [review]
timeline-mouseup.patch

Forgot to flag r?
Attachment #8579478 - Flags: review?(vporof)
(Assignee)

Updated

3 years ago
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
Blocks: 1145697
No longer blocks: 1075567, 1144799
Blocks: 1147941
No longer blocks: 1145697
Brian, are you still planning on looking at these bugs? ( https://bugzilla.mozilla.org/showdependencytree.cgi?id=1147941&hide_resolved=1 )
Priority: -- → P2
(Assignee)

Comment 6

2 years ago
(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
(Assignee)

Comment 7

2 years ago
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
(Assignee)

Comment 8

2 years ago
Created attachment 8602287 [details] [diff] [review]
timeline-mouseup.patch

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!
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 10

2 years ago
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
Last Resolved: 2 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 40
You need to log in before you can comment on or make changes to this bug.