Change in boxobject breaks autoscroll in Firefox

VERIFIED FIXED

Status

()

Firefox
General
VERIFIED FIXED
14 years ago
12 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: bz)

Tracking

({regression, testcase})

Trunk
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

14 years ago
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.
(Reporter)

Comment 1

14 years ago
Created attachment 167006 [details]
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
(Reporter)

Comment 2

14 years ago
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?
*** Bug 272154 has been marked as a duplicate of this bug. ***
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...
Assignee: nobody → firefox
Component: Layout → General
OS: Windows 2000 → All
Product: Core → Firefox
QA Contact: core.layout → firefox.general
Hardware: PC → All
Comment on attachment 167378 [details] [diff] [review]
Proposed patch

This should fix this (it fixes the testcase, at least...)
Attachment #167378 - Flags: superreview?(bugs)
Attachment #167378 - Flags: review?(mconnor)
Is this worth a relnote for seamonkey?  Every mozilla user I know uses the
autoscroll extension and will be affected at the next release.
Did someone file a bug on autoscroll to fix the extension?

If it gets fixed, I'll be willing to relnote that.
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?
*** Bug 272581 has been marked as a duplicate of this bug. ***

Comment 12

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

Comment 13

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

Updated

14 years ago
Keywords: aviary-landing

Comment 14

14 years ago
I'll have a look at it. Thanks for the info.
(Reporter)

Comment 15

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

Comment 16

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

Comment 17

14 years ago
*** Bug 273454 has been marked as a duplicate of this bug. ***
Comment on attachment 167378 [details] [diff] [review]
Proposed patch

toolkit/content does not yet require r+sr, this is good to go for checkin.
Attachment #167378 - Flags: superreview?(bugs)
Attachment #167378 - Flags: review?(mconnor)
Attachment #167378 - Flags: review+
*** Bug 273802 has been marked as a duplicate of this bug. ***

Comment 20

14 years ago
-> bz.
Assignee: firefox → bzbarsky

Comment 21

14 years ago
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
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Keywords: aviary-landing
Resolution: --- → FIXED
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

14 years ago
OK. Sorry.

Updated

12 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.