1226912 - Measuring tool doesn't remove 'mouseleave' event listeners

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

Description

[arni2033]

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

Comment 1

[Tyler Steiman]

I would love to work on this, is this only effecting non-e10s windows? I'm having trouble replicating the problem in e10s.

Comment 2

[Tyler Steiman]

Attachment: bug1226912_measuringtool_mouseleave.diff

This patch seems to fix the problem, after building `mouseleave` is no longer present.

Comment 3

[Tyler Steiman]

Attachment: screen.png

Screenshot after building with patch.

Comment 4

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

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.

Comment 5

[arni2033]

(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

Comment 6

[Tyler Steiman]

I'm assuming this would be reviewed after the holiday season?

Comment 7

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

(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.

Comment 8

[Tyler Steiman]

Any news on this?

Comment 9

[Matteo Ferretti [:zer0] [:matteo]]

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.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/98d1a38f79a1

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98d1a38f79a1

Comment 12

[Mayur Patil]

[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

[Nazir Ahmed Sabbir [:NaSb]]

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