591554 - flash plugin briefly appears in chrome when scrolling

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Timothy Nikkel (:tnikkel)]

Open a youtube flash video. Scroll up and down, the plugin briefly appears in the chrome before redrawing.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Redraw after updates

The key here is that the window clip region needs to be updated before the window is moved, and the invalidation for SetWindowRgn needs to be sent after the window is moved. If you invalidate before the position is updated you'll draw into chrome when scrolling the plugin into view (because the window region has not yet been moved into the content area). If you update the window region after setting the position, as we're currently doing, the window will move over the chrome area with the wrong clip region. Causing this bug.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #1)
> The key here is that the window clip region needs to be updated before the
> window is moved, and the invalidation for SetWindowRgn needs to be sent after
> the window is moved. If you invalidate before the position is updated you'll
> draw into chrome when scrolling the plugin into view (because the window region
> has not yet been moved into the content area). If you update the window region
> after setting the position, as we're currently doing, the window will move over
> the chrome area with the wrong clip region. Causing this bug.

Shouldn't we be calling w->SetWindowClipRegion(configuration.mClipRegion, PR_TRUE) before moving the plugin, and then calling w->SetWindowClipRegion(configuration.mClipRegion, PR_FALSE) afterwards? That's what the aIntersectWithExisting parameter was designed for.

I don't see what aNeedsRedraw is for here; we'll never set it since no-one's passing PR_TRUE for aIntersectWithExisting. SetWindowClipRegion will always exit in the first if statement.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

So SetWindowClipRegion should store PR_FALSE in *aNeedsRedraw along the paths where the region changed, and the parameter should be called aRegionChanged. That would make this more understandable.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My first paragraph of comment #2 still applies.

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: SetWindowClipRegion twice as intended

Comment 6

[Bas Schouten (:bas.schouten)]

http://hg.mozilla.org/mozilla-central/rev/82e5ae3dc59b

Comment 7

[[not reading bugmail]]

I have a few questions on this.  Was this bug the same thing as bug 590568?  

Its interesting that I filed bug 590568 3 days before this was fixed, but this fix didn't fix D3D9.  Did it add to the problem in bug 590568?  

I also think Tim is right in bug 626245 that this bug made plugin bouncing worse, which is subtle before, but it was there.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #7)
> I have a few questions on this.  Was this bug the same thing as bug 590568?

No.