Closed
Bug 1226912
Opened 7 years ago
Closed 7 years ago
Measuring tool doesn't remove 'mouseleave' event listeners
Categories
(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)
DevTools Graveyard
Graphic Commandline and Toolbar
Tracking
(firefox45 affected, firefox46 fixed)
RESOLVED
FIXED
Firefox 46
People
(Reporter: arni2033, Assigned: ma.steiman)
Details
(Whiteboard: [good first bug][lang=js])
Attachments
(2 files)
1.02 KB,
patch
|
zer0
:
review+
|
Details | Diff | Splinter Review |
187.08 KB,
image/png
|
Details |
My Info: Win7_64, Nightly 45, 32bit, ID 20151119030404 STR: 1. Open devtools, click "Measure a portion of the page" button several times 2. Open Browser Toolbox, inspect current <browser> (content area) Result: 'mouseleave' event handlers aren't removed > screenshot: https://dl.dropboxusercontent.com/s/3rqbdz4xw9wr8f8/screenshot%20-%20Measuring%20tool%20doesn%27t%20remove%20%27mouseleave%27%20event%20listeners.png?dl=0 Expectations: function "destroy()" should remove 'mouseleave' event handler as well > http://mxr.mozilla.org/mozilla-central/source/devtools/server/actors/highlighters/measuring-tool.js#213
Assignee | ||
Comment 1•7 years ago
|
||
I would love to work on this, is this only effecting non-e10s windows? I'm having trouble replicating the problem in e10s.
Flags: needinfo?(arni2033)
Assignee | ||
Comment 2•7 years ago
|
||
This patch seems to fix the problem, after building `mouseleave` is no longer present.
Assignee | ||
Comment 3•7 years ago
|
||
Screenshot after building with patch.
Comment 4•7 years ago
|
||
Comment on attachment 8696121 [details] [diff] [review] bug1226912_measuringtool_mouseleave.diff Review of attachment 8696121 [details] [diff] [review]: ----------------------------------------------------------------- Thnks, This looks good to me, but Matteo should review this to be sure.
Attachment #8696121 -
Flags: review?(jwalker) → review?(zer0)
(In reply to Tyler Steiman from comment #1) > is this only effecting non-e10s windows? I'm having trouble replicating the problem in e10s. That's a good note. In e10s-mode: 1) Browser Toolbox doesn't list those event listeners (all of them). That's probably because those are event listeners registered in remote process. 2) When I move mouse from content area, the measuring tooltip doesn't disappear. So "mouseleave" listener either doesn't fire OR operates bad. IMO this should be checked properly in separate bug Just for the record: I'm not a guy who knows much about devtools code; I just read it sometimes
Flags: needinfo?(arni2033)
Assignee | ||
Comment 6•7 years ago
|
||
I'm assuming this would be reviewed after the holiday season?
Flags: needinfo?(jwalker)
Comment 7•7 years ago
|
||
(In reply to Tyler Steiman from comment #6) > I'm assuming this would be reviewed after the holiday season? I really hope not - Matteo should be on to it shortly.
Flags: needinfo?(jwalker)
Comment 9•7 years ago
|
||
Comment on attachment 8696121 [details] [diff] [review] bug1226912_measuringtool_mouseleave.diff Review of attachment 8696121 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the very late response; Looks good to me. I designed the code for the new highlighters to avoid such things in the future.
Attachment #8696121 -
Flags: review?(zer0) → review+
Updated•7 years ago
|
Flags: needinfo?(jwalker)
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 10•7 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/98d1a38f79a1
Keywords: checkin-needed
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/98d1a38f79a1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment 12•6 years ago
|
||
[bugday-20160323] Status: RESOLVED,FIXED -> VERIFIED Comments: Test Successful. Component: Name Firefox Version 46.0b9 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: destroy() has removed the mouseleave event handler from series of event listeners. Actual Results: As expected
Comment 13•6 years ago
|
||
I've managed to reproduce this bug on Nightly 45.0a1 (2015-11-21) (Build ID: 20151121030232) on Linux, 64 Bit. This Bug is now verified as fixed on Latest Firefox Nightly 50.0a1 (2016-06-29) Build ID: 20160629030209 User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 OS: Ubuntu 14.04 ; inux 3.19.0-61-generic
QA Whiteboard: [bugday-20160629]
Updated•4 years ago
|
Product: Firefox → DevTools
Updated•4 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•