Closed Bug 463042 Opened 16 years ago Closed 16 years ago

smooth scrolling isn't working with the scrollbar or the keyboard arrows

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: gatellie, Assigned: mstange)

References

(Depends on 1 open bug)

Details

(Keywords: fixed1.9.1, regression)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; fr; rv:1.9.1b2pre) Gecko/20081104 Minefield/3.1b2pre (like Firefox/3.0)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; fr; rv:1.9.1b2pre) Gecko/20081104 Minefield/3.1b2pre (like Firefox/3.0)

Since 2008-11-02 smooth scrolling isn't working anymore in minefield if you click on the scrollbar or if you press the keyboard arrows or the spacebar.
(but it's still working with the scroll wheel) 

Reproducible: Always

Steps to Reproduce:
1.Select "use smooth scrolling" in the preferences.
2.Open any long web page.
3.Press spacebar or down arrow or the scrollbar.
Actual Results:  
Scrolling is not smooth.

Expected Results:  
Scrolling should be smooth.
Is a regression from Bug 457864.
Blocks: 457864
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Version: unspecified → Trunk
Flags: blocking1.9.1?
Assignee: nobody → mstange
Status: NEW → ASSIGNED
This makes NS_VMREFRESH_SMOOTHSCROLL the default behavior for "up" and "down" and "page up" and "page down" keys.  Perhaps NS_VMREFRESH_SMOOTHSCROLL should be renamed NS_VMREFRESH_INCREMENTALSCROLL or NS_VMREFRESH_PARTIALSCROLL or something else, since "smooth" or "not smooth" is determined by the pref, not the args.  I left ScrollByWhole (top/home/bottom/end keys) to not use this arg, because I believe most other apps scroll immediately on those keystrokes.
Is this bug already fixed in trunk?
(In reply to comment #4)
> Is this bug already fixed in trunk?

No, otherwise this bug's status would be RESOLVED FIXED.

(In reply to comment #3)
Thanks for the patch, Neil!

> don't allow async behavior to short-circuit incremental scrolling behavior.

Yep, I found that bug as well.

> This makes NS_VMREFRESH_SMOOTHSCROLL the default behavior for "up" and "down"
> and "page up" and "page down" keys.

I don't think we want NS_VMREFRESH_SMOOTHSCROLL to be the default behaviour for ScrollByLines and ScrollByPages; see bug 457864 comment 15. I think we should rather change the callers.
I already have a patch that does this; I haven't posted it yet because it would break some automated tests. They need to be changed.

> Perhaps NS_VMREFRESH_SMOOTHSCROLL should
> be renamed NS_VMREFRESH_INCREMENTALSCROLL or NS_VMREFRESH_PARTIALSCROLL or
> something else, since "smooth" or "not smooth" is determined by the pref, not
> the args.

I like INCREMENTALSCROLL. Moreover I think we should use _SCROLL instead of _VMREFRESH because these flags don't have anything to do with view refresh anymore.
Attached patch fix v1Splinter Review
Attachment #347304 - Flags: review?(roc)
Flags: blocking1.9.1? → blocking1.9.1+
Comment on attachment 347304 [details] [diff] [review]
fix v1

+  if (!(aUpdateFlags & NS_VMREFRESH_DEFERRED) &&
+      !(aUpdateFlags & NS_VMREFRESH_SMOOTHSCROLL)) {

if (!(aUpdateFlags & (NS_VMREFRESH_DEFERRED | NS_VMREFRESH_SMOOTHSCROLL)))
Flags: blocking1.9.1+ → wanted1.9.1+
Attachment #347304 - Flags: approval1.9.1?
Comment on attachment 347304 [details] [diff] [review]
fix v1

a191=beltzner
Attachment #347304 - Flags: approval1.9.1? → approval1.9.1+
pushed: http://hg.mozilla.org/mozilla-central/rev/cfc553938038
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
pushed to mozilla-1.9.1 (without the change that caused bug 468167):
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/506b19773776
Keywords: fixed1.9.1
Flags: in-testsuite+
The test I pushed was for bug 468167. This bug hasn't got a test yet.
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: