76085 - Plugins newly exposed area isn't invalidated after scrolling

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect, P2)

Description

[hirata masakazu]

There was a bit similar bug 73176 duped to 73406, which has been resolved fixed.
2001041308 Mac trunk.
Goto URL.
After loading is complete, move the scrolling tab up and down.
Result: iframe at the lower left of the window is out of synch, and some parts
never get refreshed.
Expected result: everything in the window should scroll together.

Comment 1

[Jacob Kjome]

Everything works just fine at this link as far as I can tell with build
2001043004 on win2k

Jake

Comment 2

[hirata masakazu]

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

Comment 3

[karnaze (gone)]

Reassigning to savulov for analysis/triage.

Comment 4

[Mike Pinkerton (not reading bugmail)]

this also happens on the front page of foxnews.com. it's pretty bad, and needs
to be fixed for beta.

who should get this bug?

Comment 5

[Mike Pinkerton (not reading bugmail)]

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

Comment 6

[Mike Pinkerton (not reading bugmail)]

missed some cc's. pulling over from dupe bug.

Comment 7

[Alexandru Savulov]

checked this and found out it happens on flash5 containing pages. continue 
checking with other plug-ins.

Comment 8

[Peter Lubczynski]

Is this a plugin bug or iframe bug? Can someone attach a minimal testcase? 
Hmm...lots of keywords, important...

cc:ing Brian Nesse. I recall you helping fix something similar for RTM.

Comment 9

[Alexandru Savulov]

Reassigned to Peter. I think it has something to do with the UI messages (and 
differences between macOS an windows), i think that some of the events triggered 
on windows are not triggered on mac (plug-in eating important UI messages 
maybe). It does not appear to me that it has something to do with sync... rather 
the painting is not done at all for some regions. Try with different flash5 
containing pages and see how they act. (i tried with disney.com, 
weather.com,..) Also see if the flash4 plug-in causes the same bug.

Comment 10

[Peter Lubczynski]

Okay, I've simplifed this:

1) Reisze your browser so that it's height is half of normal. 
2) Visit Disney.com
3) Start to scroll, slowly.

Notice that the plugin bits on the screen are correctly moved up. However, the 
newly exposed plugin area isn't being told invalidate.

One way to debug this would be to attach npspy logs of 4.x and Mozilla in 
scrolling a simple testcase.

Making mozilla0.9.2 as that sounds reasonable.
Chaging summary from: scrolling of iframe out of synch from rest of the window 
content

Comment 11

[Simon Fraser [no longer active]]

Did the fix for bug 31032 affect this?

Comment 12

[Peter Lubczynski]

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

Comment 13

[Brian Nesse (gone)]

I backed out the patch from bug 31032 with no obvious difference in
functionality. The problem appears to lie elsewhere.

Comment 14

[Brian Nesse (gone)]

This bug has regressed somewhat between the 2001051808 and the 2001052108
builds. In the 2001051808 build, resizing the window or hiding and showing the
window would cause an update to occur. As of the 2001052108 build resizing the
window (larger) does not cause a refresh of the exposed area. Hiding and showing
the window simply leaves a large empty white rectangle where the flash movie
should be. Scrolling around in the small window fails in both cases.

Comment 15

[Peter Lubczynski]

Another regression but I think we have a handel on it. cc:ing Chris Karnaze

If we get a fix, is this beta-stopper, branch material?

Comment 16

[Mike Pinkerton (not reading bugmail)]

i'd say this was good branch material, but you probably knew that already ;)

Comment 17

[Peter Lubczynski]

Okay, let me know how this patch works on your machine. It's backing out a fix 
for another bug. It should make it like it was before.

Comment 18

[Peter Lubczynski]

Attachment: patches to back out fix for bug 64645

Comment 19

[Mike Pinkerton (not reading bugmail)]

won't backing that out cause other problems to reappear? i'm confused.

Comment 20

[Peter Lubczynski]

Attachment: karnaze's patch (please try)

Comment 21

[Peter Lubczynski]

I've been running with the patch for the past few hours and it looks great on
both Mac and Windows. r=peterl

Seeking super review....

Comment 22

[Mike Pinkerton (not reading bugmail)]

do i have to apply both patches? running with just the 2nd patch doesn't fix it.

Comment 23

[Mike Pinkerton (not reading bugmail)]

i can't apply both of these patches (i get conflicts), so i'm assuming that they
aren't meant to be applied together.

help?

Comment 24

[Peter Lubczynski]

Damm! Still needs work! Doh..tried a bunch of sites but the actual test URL. 

Comment 25

[Mike Pinkerton (not reading bugmail)]

foxnews is better, but still not perfect (scrolling with the thumb in realtime
causes some odd jumping on their sidebar when you reach the bottom). weather.com
is almost even more broken :( much more jumping while scrolling live.

Comment 26

[Peter Lubczynski]

Try applying this patch freshly:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37415

Should fix weather.com and all the rest, but, bug 64645 will be reopned. That
patch backs out the changes. But, that bug isn't anywhere as serious as this, if
we can't have both.

Shouldn't the milestone be mozilla0.9.1?

Comment 27

[Mike Pinkerton (not reading bugmail)]

peter, i'm sure i must be doing something different. i applied that last patch
you referenced and rebuilt layout. going to 

http://www.weather.com/weather/local/94043

and scrolling does not look any better (or worse) than it did before. the 10-day
forcast still doesn't repaint correctly.

Comment 28

[Brian Nesse (gone)]

With Karnaze's patch (attachment 37434 [details] [diff] [review]) I see a distinct improvement. With it
the disney.com page only fails to update correctly when it is clipped
horizontally. If you make the window short but wide, it looks great. Same thing
at weather.com. foxnews is still wacky, even at full width however. I believe
this is definitely a step in the right direction.

Comment 29

[Peter Lubczynski]

Yes, I'm getting similar results trying this again (something was waked last 
night). karanze's patch does seem to make most sites better. However, the 
problems at disney.com seem to be there even with completely backing out the 
changes (patch #1). 

I'm reassigning this bug to karnaze as he's on top of it.

Comment 30

[Brian Nesse (gone)]

Yes, sorry if I didn't make that clear. The bug has always been there. It just
got worse between the bits I referenced. The current patch seems to fix the
additional regresssions. Now we need to locate the original problem.

Comment 31

[Peter Lubczynski]

r=peterl

Comment 32

[Marc Attinasi]

sr=attinasi

Comment 33

[karnaze (gone)]

To avoid confusion, we are trying to get the 1st patch checked in (not the 2nd), 
which will put things back the way they were before the patch in bug 64645 was 
checked in. This is necessary to fix other mac bugs which the patch to bug 64645 
caused (e.g. bug 81362). 

Comment 34

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 35

[karnaze (gone)]

The 1st attachment has been checked in and undoes the patch in bug 64645. Since 
this bug was opened before bug 64645 was checked in, I'm reassigning this to 
bnesse, moving to m0.9.3 and removing "rtm stopper" from the whiteboard.

Comment 36

[Mike Pinkerton (not reading bugmail)]

i still think this is an rtm stopper. These top100 sites look like crap, with or
without these patches.

Comment 37

[Brian Nesse (gone)]

Removing some keywords which no longer apply to the current status, timeframe, etc.

Comment 38

[Brian Nesse (gone)]

So the odd thing about this seems to be that the nsObjectFrame gets update
events while the horizontal scrollbar thumb is within 8 pixels of the leftmost
position. Beyond 8 pixels, no update events are ever sent to the nsObjectFrame.

With the vertical scrollbar, update events are sent to the nsObjectFrame
whenever the thumb is within 8 pixels of the top or bottom extent, but nowhere
in between.

Additionally, the events only come when I am moving the scrollbar away from the
end in question, not when I move towards it.

Does this mean anything to anybody? Is 8 pixels a magic number for scrolling?

If anyone wants to try and repro this, I'm currently using
http://slip/shrir/flashedit.html as a testcase because it's a very simple page.

Comment 39

[R.K.Aa.]

If I turn the number 8 around 45 degrees, it could symbolise the approximate
width of pages with objects (bug 86075, caused by patch in attachment 37415 [details] [diff] [review])

Comment 40

[Jacek Piskozub]

Patch 37415 caused a regression - bug 86075 (All/All, top100, 8 dups, critical
to 0.9.2). Adding a one line patch 39455 cures the regression.

I have no Mac to check if this patch-to-patch regresses this bug or bug 64645.
Could anyone verify that? I asked already Peter Lubczynski for his opinion on
the issue.

Comment 41

[Brian Nesse (gone)]

There is a tie in to this patch, but not the one that you think. As per my 2001-
06-01 10:43 comments there was a regression from the patch in bug 64645. It 
didn't cause this bug, it simply made it worse by adding another case in which 
the screen failed to update correctly. IIRC either patch attached to this bug 
eliminated that problem, but the one that was checked in also eliminated 2 other 
bugs relating to events not going to plugins.

Comment 42

[Brian Nesse (gone)]

After going round and round in circles on this for some time now... I'm still
not sure if it is a frame clipping problem, or a widget clipping problem. It
turns out that the 8 pixel thing I mentioned above seems to be a parent of some
sort. The plug-in widget is located at 8,8 in a parent which is 16 pixels larger
than the plug-in. Using the example of a scroll along the horizontal axis. As
long as 1 "x" pixel of the parent is in the visible region, updates are
processed. As soon as that pixel is scrolled out of the visible region, the
nsObjectFrame stops getting paint events... probably because the parent
considers itself totally hidden.

My best guess at this right now is to make the nsObjectFrame a
nsScrollPositionListener (like the CanvasFrame). Unfortunately I believe this is
more of a work around fix than a proper solution.

This really needs to be pursued by someone who has a much better understanding
of the layout engine than I do. Based on the component descriptions, this seems
like it should probably go to the compositor group.

Comment 43

[hirata masakazu]

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

Comment 44

[Kevin McCluskey (gone)]

Moving to Mozilla0.9.6

Comment 45

[Kevin McCluskey (gone)]

Reassinging to Peter.

Comment 46

[Peter Lubczynski]

The testcase no longer shows this bug, but I think it's still happening.
http://www.weather.com/weather/local/02446

Comment 47

[Brian Nesse (gone)]

Use http://slip/shrir/flashedit.html it still exhibits the undesirable behavior.

Comment 48

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 49

[Brian Nesse (gone)]

Taking as this will be fixed by the patch in bug 104445.

Comment 50

[Brian Nesse (gone)]

Fixed by the checkin for bug 104445.