271702 - Change in boxobject breaks autoscroll in Firefox

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Martijn Wargers (dead)]

Not sure if this is actually a bug the boxobject (it could be actually something
that is fixed), but I'm not sure. If it is not, it should be filed under
Firefox, I guess.

I'll attach a testcase.

The autoscroll image gets displayed too low, when you've scrolled down a bit on
a page and then use the autoscroll function (by middle clicking).

I believe that is because the boxObject.screenY propery now changes when you
scroll down a page. I guess this could be actually correct behavior. It seems
like previously the viewport position was used.

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

This more or less copied from the showAutoscrollMarker method in browser.xml
from Firefox.

This used to work in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041123 7:37am

But it works not well anymore in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041124
Firefox/0.9.1+ 8:12am

Comment 2

[Martijn Wargers (dead)]

I think the fix for bug 268576 is responsible for this change.
I get the feeling the autoscroll code needs fixing. Why isn't just evt.clientX
and evt.clientY used in that code, for example?

Comment 3

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

*** Bug 272154 has been marked as a duplicate of this bug. ***

Comment 4

[Boris Zbarsky [:bzbarsky]]

I think the problem is that the code wants coordinates "relative to the
viewport".  In which case, it probably wants to use the screenY of the window,
not the root element.

And yes, the JS in question is just broken...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 167378 [details] [diff] [review]
Proposed patch

This should fix this (it fixes the testcase, at least...)

Comment 7

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Is this worth a relnote for seamonkey?  Every mozilla user I know uses the
autoscroll extension and will be affected at the next release.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Did someone file a bug on autoscroll to fix the extension?

If it gets fixed, I'll be willing to relnote that.

Comment 9

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

http://bugzilla.mozdev.org/show_bug.cgi?id=8175
I don't know if autoscroll is still maintained.  If it isn't, then what? 
Someone has to fork it?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Probably.

Comment 11

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

*** Bug 272581 has been marked as a duplicate of this bug. ***

Comment 12

[Steffen Wilberg]

*** Bug 272695 has been marked as a duplicate of this bug. ***

Comment 13

[Steffen Wilberg]

Marc, the AiO Gestures extension is affected by this as well.

Comment 14

[Marc Boullet]

I'll have a look at it. Thanks for the info.

Comment 15

[Martijn Wargers (dead)]

*** Bug 273214 has been marked as a duplicate of this bug. ***

Comment 16

[Bill Mason]

*** Bug 273453 has been marked as a duplicate of this bug. ***

Comment 17

[Bill Mason]

*** Bug 273454 has been marked as a duplicate of this bug. ***

Comment 18

[Mike Connor [:mconnor]]

Comment on attachment 167378 [details] [diff] [review]
Proposed patch

toolkit/content does not yet require r+sr, this is good to go for checkin.

Comment 19

[Phil Ringnalda (:philor)]

*** Bug 273802 has been marked as a duplicate of this bug. ***

Comment 20

[Steffen Wilberg]

-> bz.

Comment 21

[Steffen Wilberg]

Checking in mozilla/toolkit/content/widgets/browser.xml;
/cvsroot/mozilla/toolkit/content/widgets/browser.xml,v  <--  browser.xml
new revision: 1.48; previous revision: 1.47
done

Comment 22

[Boris Zbarsky [:bzbarsky]]

Steffen, please do me a favor.  Unless there's a good reason (me asking would be
such a reason), don't check in my patches, ok?

Comment 23

[Steffen Wilberg]

OK. Sorry.