168116 - Need to implement nsIWidget::SetParent on GTK

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect, P3)

Description

[Kevin McCluskey (gone)]

this bug was broken off from bug 129034.

The platform specific widget library needs to support reparenting existing
nsIWidget's by implementing the nsIWidget::SetParent method. Until this is
Mozilla's print preview will sometimes display iframes and other elements which
require native widgets on the wrong page.

CC'ing peterl since plugin's also need this capability.

Comment 1

[Syd Logan]

Looking at it

Comment 2

[Syd Logan]

Attachment: possible patch for this

This patch assumes that what we really need to reparent is either mShell or
mMozArea. Rod, do you have a test case that you can apply this patch to? I'd
suggest having blizzard or bryner look at this patch (I cc'd him and bryner)
and see if my assumptions about mShell and mMozArea in this context are
correct.

Comment 3

[rods (gone)]

In your build open this testcase:
mozilla/layout/html/tests/printer/iframe_2nd_page/index.html

The IFrame should show correct on the 2nd page in PP

Comment 4

[rods (gone)]

I just applied the fix and looked at the test in PP and the IFrame is still on
the 1st page. BUT! I am now updating my tree to make sure I have all the other
checkins for this.

Comment 5

[Brian Ryner (not reading)]

Comment on attachment 99699 [details] [diff] [review]
possible patch for this

I don't think it makes sense to reparent mShell, since mShell represents a
toplevel window.  I think what we want to do here is reparent mSuperWin, if we
have one.  I'll let blizzard give a definitive answer though.

Comment 6

[Christopher Blizzard (:blizzard)]

There are a few things to worry about here:

o If the widget that you are reparenting has an mMozArea or an mShell it's going
to be tough.  The mMozArea and mSuperWin in that case are linked and you can't
just unparent the superwin from the mozarea and go.

o Reparenting the superwin isn't as easy as it sounds, I don't think that I've
ever tried it, frankly.  You have to worry about the focus changes and you have
to worry about making sure that it's properly destroyed.

Comment 7

[rods (gone)]

At the moment it will only be called for Print Preview so in this case the focus
won't matter.

Comment 8

[Kevin McCluskey (gone)]

I don't know if this will help simplify the solution. The SetParent will only be
called for a child window. In addition, the child window will specify a new
parent which will always be a descendant of the same top level window the child
was originally placed in. We will never move a window between top-level windows
and we will never reparent a top-level window to be a child of another top-level
window.

Comment 9

[Christopher Blizzard (:blizzard)]

That does help a bit, yeah.

Comment 10

[Kevin McCluskey (gone)]

nsbeta1-. 

Comment 11

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

This needs to be fixed to fix print preview bugs. (And eventually, to fix column
layout bugs.)

Comment 12

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 13

[Mats Palmgren (inactive)]

Attachment: Like so?

This fixes the first testcase on GTK1 and GTK2.
Robert, you said something about column layout - do you have a suggestion for a

testcase I can try?

Comment 14

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

Comment on attachment 190270 [details] [diff] [review]
Like so?

This won't be necessary in 1.9 because we'll move to no-child-windows but for
1.8 this is probably worth having. It's low risk because in the rare cases
where this gets called we currently just do something really really stupid.

Comment 15

[Mats Palmgren (inactive)]

Checked in to trunk at 2005-08-08 19:40 PDT.

-> FIXED