114667 - [viewpoint] Calls windowless plugin's SetWindow with an x,y of 0,0

Status: VERIFIED INVALID

(Plugins Graveyard :: Viewpoint Media Player, defect, P2)

Description

[Ari Berger]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    

When the embed is placed in a certain spot on our page through a TABLE, the 
browser calls our windowless plugin's SetWindow with an (x,y) of (0,0) instead 
of the correct location of the embed on the page.  Our plugin doesn't draw 
correctly, and when you force a redraw (for example, by dragging another window 
in front of the browser), the plugin draws in the top-left corner of the page.

Note that we still get mouse events for the CORRECT region.

Reproducible: Always
Steps to Reproduce:
1.Install plugin (attached)
2.View http://cole.viewpoint.com/~aberger/setwindowbug/index.html
3.

Actual Results:  when you force a redraw of the page (by dragging another 
window in front of the browser) you see the plugin drawn in the top-left corner 
of the screen.

Expected Results:  The plugin should be centered on the page.

The (x,y) passed into the plugin seems to come from 2 places:
In nsObjectFrame.cpp, DidReflow(), you get the window from:
mInstanceOwner->GetWindow(window);
The correct (x,y) is there.
You then get the offset from the parent frame:
parentFrame->GetOffsetFromView(aPresContext, origin, &parentWithView);
and use that origin as the x and y:
window->x = NSTwipsToIntPixels(origin.x + offx, t2p);
window->y = NSTwipsToIntPixels(origin.y + offy, t2p);	

This offset is 0,0, and replaces the (x,y) that we had.
Is it possible that this should instead be used as an OFFSET?
window->x += NSTwipsToIntPixels(origin.x + offx, t2p);
window->y += NSTwipsToIntPixels(origin.y + offy, t2p);	

I have attached this as a patch- it seems to work for us, both in this case, 
and in the case where the content is placed in a div tag (where the origin is 
non-zero).

Comment 1

[Ari Berger]

Attachment: Patch

Comment 2

[Ari Berger]

Sorry, I couldn't attach our plugin installer. It is too big.  Here is a link:
http://cole.viewpoint.com/~aberger/setwindowbug/viewpointplugin.zip

Comment 3

[Peter Lubczynski]

This is blocking Viewpoint and is important for embedding...

Comment 4

[Judson Valeski]

please describe the patch. can we get some r/sr on this?

Comment 5

[Peter Lubczynski]

working on it....

Comment 6

[av (gone)]

Taking

Comment 7

[David Brittain]

I'm not sure if that patch is the right fix.

I think that the problem is in this code:

        GetOffsetFromView(aPresContext, origin, &parentWithView);

        // if it's windowless we want to get the offset from the parent frame
        if (window->type == nsPluginWindowType_Drawable)
        {

          nsIWidget* aWidget;
          parentWithView->GetOffsetFromWidget(&offx, &offy, aWidget);

          nsIFrame* parentFrame;
          GetParentWithView(aPresContext, &parentFrame);

          if(parentFrame != nsnull)
            parentFrame->GetOffsetFromView(aPresContext, origin, 
&parentWithView);

        }

The first call to GetOffsetFromView correctly gets the origin, this is then 
overwritten in the next call inside the windowless only code. If instead the 
second call adds on the origin it gets then the test content works.

Patch attached - this seems to fix the test content, and my examples still work.

However, there are still problems. Try resizing the window, everything gets 
very messy then! I suspected this is related to the problem in bug 110094, 
except this time we are not getting the resize events, is it possible to listen 
for these?

Comment 8

[David Brittain]

Attachment: Alternative patch

Comment 9

[Peter Lubczynski]

DidReflow and SetWindow are both called each time the window is resized.

Comment 10

[David Brittain]

That's exactly the problem DidReflow isn't called on window resize (or 
scrolling) and so we don't get a chance to tell the plugin that the position it 
should draw at has changed via SetWindow.

Comment 11

[av (gone)]

In yesterday's build the plugin is not drawn correctly even the very first time. 
I see it only partially drawn. Is it only me?

Comment 12

[Ari Berger]

Isn't that solved by the patch?

Comment 13

[shrirang khanzode]

using today's trunk 1212 , i just see a black square on the top-center. I can't 
repro the problem mentioned:(

Comment 14

[av (gone)]

Right. Patch worked for the original test case. But I am using the reduced test 
case without outer <object> tag, just <embed>, and it does not draw correctly. 

Peter, why would object tag make a difference? Are not we supposed to ignore the 
<object> tag and go for its content?

Comment 15

[Ari Berger]

Is the reduced test case up somewhere?

Comment 16

[av (gone)]

Attachment: reduced test case

I also found that the original test case worked slightly differently when saved
locally. And I always crash on reload when using file:// type of test case.

Comment 17

[Peter Lubczynski]

What happens exactly is that the OBJECT tag is turned into an inline (or
possibly a block) in CantRenderReplaceElement. We may be incorrectly taking the
extra inline into account when positioning, but I don't see how. Can you post a
testcase? Is this only in the windowless case?

>> DidReflow isn't called on window resize...
What window are we talking about? Something isn't right here. Whenver you resize
the app window, a reflow gets triggered afterwhich you should always get a
DidReflow(). Reflow has to happen so that the HTML knows where to position and
size it'self. Without Reflow, we'd work similar to 4.x. What testcase is causing
Reflow not to happen?

If you have a debug build, you can run viewer.exe to see the frame model. Choose
to dump frames from the debug menu and you'll see the frame tree plus some
sizing and positioning info. You can also set the environement variable:
NSPR_LOG_MODULES=Plugin:5,PluginNPN:5,PluginNPP:5

Comment 18

[av (gone)]

BTW, without tables it all looks fine. Can you confirm?

I got an impression that the frame does not get reflow command on resize when 
placed inside a table cell. If I remove the table ornamentation from the test 
case nsObjectFrame::DidReflow gets called and drawing area moves appropriately 
as directed by <div align="center">.

Comment 19

[av (gone)]

Peter, if you remove the plugin and go to the test case you will see the default 
plugin in action which indeed is not windowless. It does seem to be redrawn on 
resize correctly (and it reflows visually), but I don't stop at the break point 
inside nsObjectFrame::DidReflow!

Comment 20

[Peter Lubczynski]

Your right! no Reflows either, but only if it's inside a table.

Karnaze, is there some kind of optimization made not to reflow children of tables?

Comment 21

[karnaze (gone)]

Yes. Turn on reflow debugging by setting the env var 
GECKO_DISPLAY_REFLOW_RULES_FILE to a local file of your choice and in that file 
put a line containing "* 1". You will see what reflows are happening.

In general, please don't assume that I keep up with my bug activity enough to 
respond to questions in them.

Comment 22

[Peter Lubczynski]

Okay, I just asked Kevin and here's the scoup: A resize of the client window
does not guarantee a "resize-reflow", which is what we get without the table. We
only get one of these if the table's size changes, which it doesn't in this
testcase.

So, how do we get notified about changes in position if we don't always get
reflowed? Here's two ideas (from Kevin):

1) In HandleEvent(), listen for an NS_SIZE event
2) Attach a DOM size listener to the document

Hopefully, #1 is the easier way to go, if we actually get this event.

Comment 23

[Peter Lubczynski]

Oh, another way Karnaze just suggested is overriding nsIFrame::MoveTo(), but be
careful. See html/forms/src/nsListControlFrame.cpp for an example.

Comment 24

[av (gone)]

I have tried MoveTo approach, no success :( It does not seem to be called at all 
on resize.

Comment 25

[av (gone)]

I need help on this one. Looks like we have two different problems here:

I. (no table) <center> style does not resolve into right x position on first 
page load, it is always 120 in twips which means aligned to the left. Y position 
is correct
  - resizing the page somehow gives us the right value and if we apply     
'alternative patch' it will fix it up if you do resize

II. if plugin is in the table cell we do not get reflow at all, this should be 
probably addressed separately.

I am adding some people for input.

Comment 26

[Ari Berger]

Looks to me like there are 2 different bugs here.  Our original problem is 
fixed by the "alternative patch".  Are these other problems just a related 
issue?
The first problem is that SetWindow gets the wrong coordinates.
The second is that SetWindow is not always called on resize or scroll.
If this is true, would it be possible to check in the patch for the first 
problem separately? A fix for this is holding back QA of our plugin.

Comment 27

[av (gone)]

I filed bug 116093 for the first problem and bug 116108 on not calling SetWindow 
on resize when in table cell. Is scrolling yet another issue? If we cover 
everything with this split we should probably close this one.

Comment 28

[Michael Dunn]

Marking as invalid since it is split to two bugs listed. Removing 094.

Comment 29

[shrirang khanzode]

v