See: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/browser.xml&rev=1.50&root=/cvsroot#855 Every time when the mouse is moved in Firefox, this code is executed. I don't think that is really necessary, as this event handler is only necessary when the autoscroll is enabled and active. So I think it would probably be better if a mousemove eventhandler would be attached when the middle mouse button is pressed inside the content area (and detached when autoscroll is not active anymore). I'll attach a exaggerated testcase that shows that this indeed is slowing down Firefox a bit.

This takes appr. 4497ms in current trunk Firefox build. But it takes appr. 2453ms in current trunk Mozilla build. When I remove the <handler event="mousemove"> in browser.xml of Firefox, Firefox becomes just as fast as Mozilla.

Like this? This makes Firefox behave just as fast witht the testcase as Mozilla (and the autoscroll coe still functions the way as it should be).

Good idea. Why don't you ask for review?

Comment on attachment 173121 [details] [diff] [review] patch Well, I sort of forgot about it. Anyway, it is not a very important bug.

Comment on attachment 173121 [details] [diff] [review] patch r=me, please strip the bogus whitespace >@@ -728,16 +729,34 @@ here (should be none this line) >+ >+ <method name="handlemousemove"> >+ <parameter name="evt"/> and here >- >+ >+ this.addEventListener('mousemove', this.handlemousemove, false);

Wow, that's fast. Thanks, Mike! I've done what you asked. But also, I've changed the indenting at the last code in the mousemove handler, which I'm now beginning to believe was wrong.

Mike is not on the cc list Martijn, so you may either need to cc him or ask review again (or bug him on IRC)

Comment on attachment 178487 [details] [diff] [review] patch Yes, I know Peter. I just wanted to recheck the patch, that it worked correctly (which it still seems to do). Mike, this patch looks quite different than the first one, but it is essentially the same. All it does is removing the two lines you asked for and it removes some white-space, which is the code you see at the end of this patch.

Comment on attachment 178487 [details] [diff] [review] patch >+ <method name="handlemousemove"> interCaps please (handleMouseMove), sorry I missed this >+ <parameter name="evt"/> convention is to use aEvent for method/function args, makes a lot of code clearer >+ if ((x > this._AUTOSCROLL_SNAP || x < -this._AUTOSCROLL_SNAP) || (y > this._AUTOSCROLL_SNAP || y < -this._AUTOSCROLL_SNAP)) if ((x > this._AUTOSCROLL_SNAP || x < -this._AUTOSCROLL_SNAP) || (y > this._AUTOSCROLL_SNAP || y < -this._AUTOSCROLL_SNAP)) some crazy people use 80 col editors, huge lines are bad >+ this._snapOn = false; trailing whitespace again! :) almost there! :)

Ok, I fixed those things you told me. > convention is to use aEvent for method/function args, makes a lot of code > clearer Well, there are quite some more method/functions in browser.xml that seem to make use of evt.

fix checked in

+ this.removeEventListener('mousemove', this.handleMouseMove, false); Still some whitespace here, I don't know how anal you want to be about that. :)

My editor sucks. I'll remove that whitespace when I need to update a pending patch.

Verifiying fixed using Win FF 1.5. Result: 2016ms.