Closed Bug 469613 Opened 11 years ago Closed 11 years ago

Clicks on scrollbar's blank areas (above or below the thumb) don't scroll very long pages

Categories

(Core :: Layout, defect, P1, major)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: yuki, Assigned: mstange)

Details

(Keywords: fixed1.9.1)

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081214 Shiretoko/3.1b3pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081214 Shiretoko/3.1b3pre

A click on the blank area of the scrollbar should scroll the page by a height of the viewport. In short pages, like the search result of Google, the action works correctly. But, sometimes I saw that the action doesn't work in long pages.

The problem appears on my system with recent nightly builds, and I didn't see it in the builds in the last week. It is possibly a regression.


Reproducible: Sometimes

Steps to Reproduce:
1. Start Shiretoko 3.1b3pre with a clean profile.
2. Go to a long page, like http://urasoku.blog106.fc2.com/blog-entry-570.html
3. Wait for a while, until the page is completely loaded.
4. Click the blank area below the thumb of the vertical scrollbar multiple times.
Actual Results:  
Suddenly stops to scroll. After that, clicks on the scrollbar never work like it got jammed. But, after I've scrolled the page by the mouse wheel, clicks work correctly again.

Expected Results:  
Scrolls smoothly.

When the problem appears, I was building Firefox from the source. It is possibly caused when the CPU is too busy. Of course scrolling by the mouse wheel always works correctly even if the system is slow. The problem appears only with clickings on the scroll bar.
Version: unspecified → 3.1 Branch
Another case:

1. Start Shiretoko 3.1b3pre with a clean profile.
2. Go to a long page, like http://urasoku.blog106.fc2.com/blog-entry-570.html
3. Scroll to middle of the page by scrolling of the mouse wheel, before the page is completely loaded.
4. Wait for a while, until the page is completely loaded.
5. Click the blank area below the thumb of the vertical scrollbar multiple
times.

Actual Results:  
Suddenly stops to scroll.After that, clicks on the area below the thumb never work. But, after I've clicked the area above the thumb, then, clickings on the area below the thumb work correctly again.

Expected Results:  
Scrolls smoothly.
Note:

+--+
|/\|
|  | <- the blank area above the thumb
|  |
|--|
|==| <- the thumb
|--|
|  |
|  | <- the blank area below the thumb
|\/|
+--+
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081214 Minefield/3.2a1pre

I tested the case from comment 1 but was not able to reproduce the problem.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081214 Minefield/3.2a1pre

Both cases from comment 0 and comment 1 work fine for me. That proves nothing; maybe someone else can.
Can you also reproduce it with a new profile or with another mouse?
http://support.mozilla.com/en-US/kb/Basic+Troubleshooting#Make_a_new_profile
(In reply to comment #4)

> Can you also reproduce it with a new profile?

Sorry, you already did.
Hmm... also on my system, sometimes works fine, sometimes doesn't.

Another case:

1. Start Shiretoko 3.1b3pre with a clean profile.
2. Tools => Error Console
3. Copy following code and paste it to "Code:" field:
-------------------------------------------------------------------------------
Components.classes["@mozilla.org/appshell/window-mediator;1"].getService(Components.interfaces.nsIWindowMediator).getMostRecentWindow("navigator:browser").gBrowser.addEventListener('DOMContentLoaded', function(aEvent) { for (var i = 0, maxi = 100; i < maxi; i++) { Application.console.log(aEvent.currentTarget.selectedTab.linkedBrowser.contentDocument.URL); } }, false);
-------------------------------------------------------------------------------
4. Push "Message" button on the Error Console.
5. Keeping the Error Console open, switch to the browser window and go to http://urasoku.blog106.fc2.com/blog-entry-570.html
6. Wait for a while, until the page is completely loaded.
7. Many messages appear in the Error Console.
8. Click the blank area below the thumb of the vertical scrollbar multiple
times (10 or 20 times).

The 3rd step makes CPU busy when the page is loaded on my system. With this condition, the problem often appears. My system is:

* Intel Celeron 2.4GHz
* 992MB RAM
* Windows XP

If your system has higher performance, I guess that changing "100" to a greater number possibly reproduces the problem.
> 7. Many messages appear in the Error Console.
> 8. Click the blank area below the thumb of the vertical scrollbar multiple
times (10 or 20 times).

Of course, you have to wait while the console is receiving messages. Go to 8th step after you get control back.
I reproduced only one time with 20081218035829-trunk/WinVista, we need to find the way to reproduce...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Yes, it happened also to me once, at least something similar, on the mozilla central site, I clicked repeatedly on the arrow on top but it seemed stuck. I tried to reproduce it on a new profile but I couldn't.
Markus, could Bug 463042 have caused this?
I was able to reproduce it on 5 Dec 2008, but not on 4 Dec 2008 so far, and your bug is in this range. Although I'm certain of 5 Dec, but not of 4 Dec, because it is hard for me to reproduce.
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
Version: 3.1 Branch → Trunk
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Yes, it's very likely that bug 463042 has caused this.
I succeeded in reproducing this bug with the steps from comment 6.
Assignee: nobody → mstange
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Status: NEW → ASSIGNED
Markus, any progress on this? I can get someone else to take it if you don't have time to work on it.
I think I've found the bug.
Attached patch fix v1 (obsolete) — Splinter Review
This change in bug 457864 was wrong:

   // do nothing if the we aren't scrolling.
-  if (aDestinationX == mOffsetX && aDestinationY == mOffsetY) {
+  if (aDestinationX == mDestinationX && aDestinationY == mDestinationY) {
     // kill any in-progress smooth scroll

It means that two consecutive calls ScrollTo with the same position result in no scrolling at all because the async scroll is canceled.
My patch reverts that change and adds a test.
Attachment #355818 - Flags: review?(roc)
pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/fe759e3fd895
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.2a1
this was backed out due to mochitest failures, not re-opened ?
changeset: http://hg.mozilla.org/mozilla-central/rev/0ff733711384
I'm currently looking at the test that failed (test_datepicker.xul) to find out whether it was the patch or the test that caused the failure. I haven't got a clue yet.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Another funny thing is that the test only failed on Linux.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1231500485.1231504660.29178.gz#err22
I can't reproduce the test failure in my Ubuntu VM.
Maybe we should back out the patch that caused the regression and then try to proceed from there?
That's probably the right way to go forward.
Just to make sure that it's not the new test that causes the test failure I relanded the fix without the test today and backed out again; test_datepicker was still failing.
I'll try to reproduce this in my Linux VM and if that doesn't work I'll do the backout.
This patch backs out bug 457864, bug 462793 and bug 463042.
Can't reproduce it.

I won't be able to back it out straight away --- Markus, if you can back it out, or someone else can, that would be good.
Keywords: checkin-needed
can't reproduce the problem on Linux.
Attached patch fix v2 (obsolete) — Splinter Review
Smaug discovered a problem with the original patch: Due to the early return when mDestination == aDestination, a synchronous scroll might not be synchronous when there's a pending async scroll to the same position. I've removed the early return and added a test for that case.

Let's hope that this is what caused the test_datepicker failure.
Attachment #355818 - Attachment is obsolete: true
Attachment #358212 - Flags: review?(roc)
Comment on attachment 358212 [details] [diff] [review]
fix v2

It's worse than that, right? We're actually cancelling the async scroll when we might not have reached the destination, so we'll *never* reach the destination, sync or async.

But shouldn't we do ClampScrollValues before we do this check?
Attached patch fix v3Splinter Review
(In reply to comment #27)
> (From update of attachment 358212 [details] [diff] [review])
> It's worse than that, right? We're actually cancelling the async scroll when we
> might not have reached the destination, so we'll *never* reach the destination,
> sync or async.

Yeah, but that was already fixed in the first patch (attachment 355818 [details] [diff] [review]), wasn't it?

But even the second patch (attachment 358212 [details] [diff] [review]) had a bug - we fail to reset mDestinationX/Y after we cancel the async scroll.

> But shouldn't we do ClampScrollValues before we do this check?

That too. First I considered moving the check down to after ClampScrollValues, but then I realized that at that point there's no real gain in doing an early return. So I just removed the early return completely.
There are other bail-out-when-not-scrolling checks in ScrollToImpl and Scroll, they should be enough.

I also added a third test that ensures that we're resetting mDestinationX/Y properly.
Attachment #358212 - Attachment is obsolete: true
Attachment #358376 - Flags: review?(roc)
Attachment #358212 - Flags: review?(roc)
Markus, can you check this into trunk in the next 24 hours? If I don't hear from you by then I'll check it in myself to make the beta3 freeze.
Landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/a0cf750c586a

I'll resolve this bug if the tests pass.
The linux testrun timed out before test_datepicker could run. Unfortunately I don't have time to wait for another cycle, but peterv will monitor the tree for me.
Tests passed, yay!

Roc, if you have time to land it on 1.9.1 before I can, feel free to do so.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs 191 landing]
Summary: Clickings on scrollbar's blank areas (above or below the thumb) don't scroll the page, if the page is very long. → Clicks on scrollbar's blank areas (above or below the thumb) don't scroll very long pages
Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/924078312e92

Shimoda, the new nightly is out, can you still reproduce the problem?
Flags: blocking1.9.2?
Whiteboard: [needs 191 landing]
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20090126 Shiretoko/3.1b3pre
Clicks on scrollbar work correctly on my system too.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.