116093 - [viewpoint] Windowless plugins do not always get right coordinates

Status: VERIFIED FIXED

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

Description

[av (gone)]

This bug is off-spring from bug 114667.

If you use <center><embed ...><\center> construction with the windowless plugin, 
you will see that the plugin ignores <center> tag and always drawn with (0,0) 
position. Test case to follow. Please install ViewPoint plugin for testing:
http://cole.viewpoint.com/~aberger/setwindowbug/viewpointplugin.zip

Ari, please update the plugin location if this link is obsolete.

Comment 1

[av (gone)]

...restoring the parent bug nominations.

Comment 2

[av (gone)]

Attachment: test case with <center><embed...><\center> construction

Comment 3

[Ari Berger]

Best link for our plugin is here:
http://cole.viewpoint.com/~aberger/viewpointplugin.zip
If you already have us installed and just want the latest .dll and .xpt:
http://cole.viewpoint.com/~aberger/npViewpoint.zip
http://cole.viewpoint.com/~aberger/npViewpoint.xpt

Comment 4

[av (gone)]

Attachment: patch v.1

This patch is in cvs -uw format so do not pay much attention to white spaces,
they are aligned well in the original fix.

The patch is based on patch
http://bugzilla.mozilla.org/attachment.cgi?id=61428&action=view 
from bug 114667 with one addition: with only that patch we do not get it right
after the page shows up first time, it picks up only when you start to resize
and on. I added an additional reflow command after the first reflow which seems
to fix this problem.

Please review and try.

Comment 5

[av (gone)]

Attachment: patch v.2

This version is essentially the same, I just factored out some code into
nsObjectFrame::GetWindowOrigin as I am going to reuse this code for fixing bug
116108.

Please review.

Comment 6

[David Brittain]

The patch looks good to me and works with my test content.

Comment 7

[av (gone)]

Attachment: patch v.3

Nothing dramatically new, just hide twips to pixel conversion inside
GetWindowOrigin method to make code smaller and cleaner.

Comment 8

[av (gone)]

Attachment: patch v.4

Sorry, forgot the header file which did not change but still needs to be in the
patch, I guess.

Comment 9

[av (gone)]

Interesting idea. Why do we need to set up the window in DidReflow at all? We 
will do it anyway in Paint method. No need to do it twice. This should be true 
for windowless plugins (windowed plugins still need it here) and for Mac in 
general!

Patrick? Peter?

Comment 10

[av (gone)]

Attachment: patch v.5

I need massive opinions on this one to make sure I was not too radical. I
removed a bunch of code from DidReflow and return early from it on Mac (always)
and on Windows (for windowless plugins) relying on the fact that all this
window setup action will be correctly done in Paint method.

Comment 11

[av (gone)]

Serge, would you please try this on Unix to make sure we don't break anything?

Comment 12

[serge (gone)]

it looks good to me, nothing is broken on linux.

Comment 13

[Peter Lubczynski]

Comment on attachment 62479 [details] [diff] [review]
patch v.5

r=peterl

Comment 14

[Marc Attinasi]

Comment on attachment 62479 [details] [diff] [review]
patch v.5

sr=attinasi

Comment 15

[av (gone)]

In the trunk. Nominating for 0.9.7.

Comment 16

[av (gone)]

In 0.9.4, marking fixed.

Comment 17

[shrirang khanzode]

Confirmed fixed on mozilla trunk 122603. Ari, could you please confirm the fix 
as well. Thanks! I have filed  http://bugscape.mcom.com/show_bug.cgi?id=11566 
for a problem that I see using commercial trunk builds. Cannot berify the fix on 
the embedding branch unless I get thru that problem.

Comment 18

[shrirang khanzode]

Ok, Arun clarified that viewpoint might be blocking commercial netscape builds, 
so I changed the useragent string on commercial branch build and verified this 
bug. It works fine, the cube is not drawn at (0,0) and appears centered. Verif 
using 1225 branch build.

Comment 19

[shrirang khanzode]

adding keyword 'verified0.9.4'