507402 - Flash plugin mispositioned when scrolling

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Sylvain Pasche]

Attachment: testcase

When scrolling with the wheel or up/down keys, flash plugin element sometimes doesn't scroll and appears above the surrounding text. That's a regression from bug 505896.

See testcase, I couldn't reproduce the issue with the test plugin.

Comment 1

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

Attachment: testcase#2

I can reproduce this reliably by loading this slightly modified testcase and pressing page-down then page-up --- the plugin is gone. I've removed the inner DIVs, this is really simple.

Comment 2

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

Attachment: fix

This fixes it. Because ConfigureChildren has short-circuit paths now that depend on the widget's mBounds being correct, we need to update mBounds to account for the effects of ScrollWindowEx before we call ConfigureChildren.

Comment 3

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

I'm not sure why the test plugin isn't affected by this. With wmode=window, it should be affected, but doesn't seem to be. But even if it was, we can't really write a test here, since the widget's mBounds always looks correct, the bug is that the HWND ends up in the wrong position.

Comment 4

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

This patch isn't right since we iterate over the children in aConfigurations but not other children we may have. It's also not right because ScrollWindowEx is supposed to send WM_MOVE messages to the child windows which should be updating our mBounds, so this patch shouldn't even be necessary.

Comment 5

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

Ah, but plugin window messages don't go through nsWindow::WindowProc. So that code never gets called...

Comment 6

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

Attachment: fix

Sorry Jim, this patch makes more sense.

Comment 9

[[not reading bugmail]]

This looks like its fixed, see testcase in bug 511816.

Comment 10

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

http://hg.mozilla.org/mozilla-central/rev/7af754a937d6

Comment 11

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

Bustage fix:
http://hg.mozilla.org/mozilla-central/rev/d2b9effde892

Comment 12

[[not reading bugmail]]

Needs a 1.9.2 branch push.

Comment 13

[Benjamin Smedberg]

Comment on attachment 393974 [details] [diff] [review]
fix

This doesn't need approval because it's a blocker, but I'll mark it anyway!

Comment 14

[Daniel Holbert [:dholbert]]

Looks like roc landed this on 1.9.2, as part of this push on 9/23:
http://hg.mozilla.org/releases/mozilla-1.9.2/pushloghtml?changeset=c46bf0f0d355

 main fix:    http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c46bf0f0d355
 bustage fix: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/33c9121a6bdc