Closed
Bug 591554
Opened 14 years ago
Closed 14 years ago
flash plugin briefly appears in chrome when scrolling
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta5+ |
People
(Reporter: tnikkel, Assigned: bas.schouten)
References
()
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Open a youtube flash video. Scroll up and down, the plugin briefly appears in the chrome before redrawing.
Assignee | ||
Comment 1•14 years ago
|
||
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.
(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.
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.
My first paragraph of comment #2 still applies.
Assignee | ||
Comment 5•14 years ago
|
||
Assignee: nobody → bas.schouten
Attachment #470132 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #470157 -
Flags: review?(roc)
Attachment #470157 -
Flags: review?(roc) → review+
blocking2.0: --- → beta5+
Assignee | ||
Comment 6•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 7•14 years ago
|
||
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.
(In reply to comment #7)
> I have a few questions on this. Was this bug the same thing as bug 590568?
No.
You need to log in
before you can comment on or make changes to this bug.
Description
•