Closed
Bug 1263397
Opened 8 years ago
Closed 8 years ago
The tree map should zoom at centering around the mouse pointer
Categories
(DevTools :: Memory, defect, P1)
DevTools
Memory
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: magicp.jp, Assigned: gregtatum)
Details
Attachments
(3 files, 1 obsolete file)
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 Build ID: 20160409030219 Steps to reproduce: 1. Start latest Nightly 2. Go to any sites (e.g. about:home) 3. Open DevTools > Memory 4. Select Tree Map view (default) 5. Take snapshot 6. Zoom-in on the tree map at centering around the mouse pointer using mouse wheel Actual results: Cannot zoom at centering around the mouse pointer. Expected results: The tree map should zoom at centering around the mouse pointer.
Has STR: --- → yes
status-firefox48:
--- → affected
Component: Untriaged → Developer Tools: Memory
OS: Unspecified → All
Hardware: Unspecified → All
Updated•8 years ago
|
Flags: needinfo?(gtatum)
Priority: -- → P1
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb0126f5cab8
Assignee | ||
Comment 2•8 years ago
|
||
Argh, this should fix it. Was measuring off of the new width/height, not the old width/height.
Attachment #8741121 -
Flags: review?(nfitzgerald)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gtatum
Comment 3•8 years ago
|
||
Comment on attachment 8741121 [details] [diff] [review] The tree map should zoom at centering around the mouse pointer Review of attachment 8741121 [details] [diff] [review]: ----------------------------------------------------------------- How can we test the zooming better? Zooming has been a little finicky, and I don't feel comfortable landing new changes without a testing story for the zooming.
Attachment #8741121 -
Flags: review?(nfitzgerald)
Assignee | ||
Comment 4•8 years ago
|
||
The errors have been incorrect implementations on my part. I can create a browser test that measures exact DOM changes.
Assignee | ||
Comment 5•8 years ago
|
||
This patch adds specific units to the tests to measure specifically what changed. Before it was just checking general relationships, like if the zoom was in between certain values. This should lock in the current behavior. The only other way to test this behavior would be to redo the calculations for centering on the test. I'm not sure if you feel that it warrants that, or if locking in specific values for the working behavior will be enough.
Attachment #8741357 -
Flags: review?(nfitzgerald)
Assignee | ||
Updated•8 years ago
|
Attachment #8741121 -
Attachment is obsolete: true
Flags: needinfo?(gtatum)
Assignee | ||
Comment 6•8 years ago
|
||
This video demonstrates the fixed zooming behavior.
Comment 7•8 years ago
|
||
(In reply to Greg Tatum [:gregtatum] [@gregtatum] from comment #6) > Created attachment 8741358 [details] > zoom-behavior.mov > > This video demonstrates the fixed zooming behavior. Great!
Comment 8•8 years ago
|
||
Comment on attachment 8741357 [details] [diff] [review] The tree map should zoom at centering around the mouse pointer Review of attachment 8741357 [details] [diff] [review]: ----------------------------------------------------------------- \o/ ::: devtools/client/memory/test/browser/browser_memory_tree_map-02.js @@ +120,5 @@ > + "Translate X was affected by the mouse position"); > + ok(floatEquality(dragZoom.translateY, 18.18181818181817), > + "Translate Y was affected by the mouse position"); > + is(dragZoom.zoom, 0.2, "Zooming starts at 0"); > + } Thank you very much! This let's me sleep at night :)
Attachment #8741357 -
Flags: review?(nfitzgerald) → review+
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df91987438f3
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/dece9231ef6c
Keywords: checkin-needed
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dece9231ef6c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•