301152 - [10.4] Scrollbar gets messed up when scrolling while loading a page

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect, P2)

Description

[Echidnae]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050717 Camino/0.9a2
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050717 Camino/0.9a2

When loading a page, such as http://cnn.com and scrolling while the page is
loading, the scroll bar becomes distored.  Looks like this first appeared in the
20050716 nightly.

Reproducible: Always

Comment 1

[Echidnae]

Attachment: messed up scrollbar

messed up scrollbar at http://cnn.com

Comment 2

[Simon Fraser [no longer active]]

Regression from bug 166932?

Comment 3

[Will Kendall]

Attachment: Another example, this time from Spymac.com

Comment 4

[Simon Fraser [no longer active]]

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

Comment 5

[antispam]

I just saw this behavior as well, so confirming for Camino Version 2005071808
(0.9a2)

Thanks for all your hard work.

Comment 6

[Kurt Denison]

I have this same problem.  But very frequently with sites.  gettyimages,
gizmodo, to name a couple.  What is going on here?  As soon as i click out of
the browser it goes back to normal.  I could sent an image if requested.

Comment 7

[Phil]

Yup - I've seen a lot of this also especially on pages with lots of images that
continue to load after the text of the page has already been displayed.

Comment 8

[Simon Fraser [no longer active]]

Attachment: Testcase

Note that you have to actively scroll the document while it's being loaded to
see this.

Comment 9

[Simon Fraser [no longer active]]

My patch from bug 166932 regressed this. I think the issue is that the
optimization there breaks the drawing of non-NSQuickDrawViews (such as the
NSScroller) which are parented in NSQuickDrawViews.

Comment 10

[Jon C. Thomason]

I actually see similar results in Safari from time to time.
Just not on the same sites: theirs tend to involve frames.

The scroll bar will leave pieces of larger thumbs whenever
significantly shortening the thumb to reflect more content
(as shown in the attachments).  Safari will also sometimes
erase the left half of a vertical scrollbar for some reason.

Mac OS X 10.4.2, Safari 412.2.  I think it's a Tiger bug.

Comment 11

[Simon Fraser [no longer active]]

Attachment: Patch to call -setNeedsDisplay after updating the scrollbar position/size

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 191921 [details] [diff] [review]
Patch to call -setNeedsDisplay after updating the scrollbar position/size

r=pink

Comment 13

[Simon Fraser [no longer active]]

Checked in.

Comment 14

[Samuel Sidler (old account; do not CC)]

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

Comment 15

[Samuel Sidler (old account; do not CC)]

Reopening. Problem still exists and was never fixed according to checks in bug
304407

Comment 16

[Samuel Sidler (old account; do not CC)]

Since this checkin has to be made in the widget directory and not the
Camino-only directory, should this block 1.8b4?

Flagging blocking1.8b4?

Comment 17

[Mike Pinkerton (not reading bugmail)]

this is much better than it used to be, i only see it rarely.

Comment 18

[Chris Lawson (gone)]

This seems to have gotten worse recently.

cl

Comment 19

[froodian (Ian Leue)]

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

Comment 20

[Samuel Sidler (old account; do not CC)]

This is still seen quite a bit by a lot of us. We should find a regression range and see if we can see what's going on. 

Comment 21

[Maximilian Ruderburg [:max] (Arctic Revolution)]

Is anyone on current trunk able to reproduce this? With the landing of bug 311304, this might be fixed.

Comment 22

[davedit]

(In reply to comment #21)
> Is anyone on current trunk able to reproduce this? With the landing of bug
> 311304, this might be fixed.
> 

I'm still able to reproduce this bug. I believe the patch for 311304 landed on 11/9 on the trunk, and I was able to reproduce with Version 2006111605 (1.2+). I'll post some attachments.

Comment 23

[davedit]

Attachment: Messed up scrollbar while scrolling at www.gametrailers.com

Comment 24

[davedit]

Attachment: Messed up scrollbar while NOT scrolling at www.macupdate.com

Comment 25

[davedit]

(In reply to comment #24)
> Created an attachment (id=246127) [edit]
> Messed up scrollbar while NOT scrolling at www.macupdate.com
> 

I should clarify on this one. Sometimes on MacUpdate, the scrollbar will appear before the page finishes loading, but the size of the scrollbar does not readjust itself for the full size of the page once it finishes loading. If I then scroll down, the true-size scrollbar will appear and will scroll over the left over artifacts.

Comment 26

[Stuart Morgan]

Attachment: Testcase 2

I found a new reproducible testcase; adding content on the fly via JS makes it happen every time (and makes it clear that we aren't redrawing the bar in all cases we should be).

Comment 27

[Stuart Morgan]

Attachment: fix

This was another case like the general nsChildView problem Simon fixed a while back; we were calling setNeedsDisplay from within the bowels of Cocoa drawing, which it doesn't like. I switched it over to Invalidate, which has the logic of figuring out when the invalidate needs to be delayed.

Comment 28

[Chris Lawson (gone)]

The patch doesn't fix things on trunk for me, even after a distclean.

cl

Comment 29

[Stuart Morgan]

Attachment: correct fix

Thanks cl. Apparently it works better when one copies libwidget_mac.dylib after every change+rebuild in widget, not just the first one :P

This is the preliminary patch that I was actually testing when I wrote the last iteration as a misguided attempt at efficiency based on misunderstanding the size of the gecko scrollbar view. Invalidating all of it actually is the right thing to do, which makes the patch much simpler.

Comment 30

[Simon Fraser [no longer active]]

Comment on attachment 251188 [details] [diff] [review]
correct fix

sr=me but maybe use PR_FALSE?

Comment 31

[Stuart Morgan]

Oops, that should indeed be PR_FALSE. I'll fix on checkin.

Comment 32

[Stuart Morgan]

Requesting MOZILLA_1_8_BRANCH approval; note that Camino is the only product using this code on the 1.8 branch.

Comment 34

[Chris Lawson (gone)]

I can confirm the new patch works, too. Not that you needed it, but r=me.

cl

Comment 35

[Chris Lawson (gone)]

Comment on attachment 251188 [details] [diff] [review]
correct fix

Really requesting branch approval, since it doesn't appear the flag actually got set, and setting the addl-review flag just for grins.

Comment 36

[Chris Lawson (gone)]

Comment on attachment 251188 [details] [diff] [review]
correct fix

er, oops. Meant just to set 1.8, not 1.8.0.

cl

Comment 37

[Stuart Morgan]

Ah, I see. I didn't see approval since I was looking on the bug page, so I set blocking? on the bug instead.

Comment 38

[Jay Patel [:jay]]

Comment on attachment 251188 [details] [diff] [review]
correct fix

Approved for 1.8 branch, a=jay for drivers.

Comment 39

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH