Closed
Bug 69922
Opened 23 years ago
Closed 23 years ago
intrinsic sizing is broken in embedded apps
Categories
(SeaMonkey :: General, defect, P1)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9
People
(Reporter: danm.moz, Assigned: arik)
References
Details
(Keywords: embed)
Attachments
(3 files)
1.71 KB,
patch
|
Details | Diff | Splinter Review | |
10.46 KB,
patch
|
Details | Diff | Splinter Review | |
11.29 KB,
patch
|
Details | Diff | Splinter Review |
Reported by -- errrr -- I forget who. Conrad? Windows sized intrinsically don't change their size in embedded apps; even those apps that have window sizing hooked up.
Updated•23 years ago
|
Priority: -- → P3
Comment 1•23 years ago
|
||
I'm going to take a wild guess here as to why this is happening. I'll bet that since we changed the interface to nsIEmbeddingSiteWindow the code that does the resizing assumes that you're still using the old interfaces and can't QI the object to it and can't therfore resize. Possible? I haven't looked at any code so I'm just flying blind here. :)
Comment 2•23 years ago
|
||
No, that's not it. Intrinsic sizing has never worked. Also, it's not exactly broken but just not implemented. The embedding app has to know not only how to do intrinsic sizing (by calling SizeToContent()) but when (if the window is chrome, and if it's never had its size explicitly set). The reason this bug exists is to move SizeToContent to a public iface OR to have the webbrowser lib handle sizing to content automatically so the embeddor doesn't have to deal with it.
Comment 3•23 years ago
|
||
Strange. This used to work for me when I was using the old interfaces. But at that time I was returning an nsIDOMWindowInternal and I was using the appshellservice ( ASS ) to get XUL windows. I suppose those support intrinsic resizing. I'm still confused as to why the external interfaces require specific support to handle intrinsic resizing. To the window it just looks like a |SetDimensions| and the flags for the chrome around the window are passed in as normal. For example, my embedding widget looks fine except for the size. The content looks great and seems to load just fine. Is it as simple as finding the inner DOM window and calling |SizeToContent|? I've already got a progress listener attached to the window and I think that should report when the load finishes. I might have to attach to the inner window in the chrome case but I haven't played with it enough to know yet.
Comment 4•23 years ago
|
||
> Strange. This used to work for me when I was using the old interfaces. But at > that time I was returning an nsIDOMWindowInternal and I was using the > appshellservice ( ASS ) to get XUL windows. I suppose those support intrinsic > resizing. That's right - XUL windows with appshell do the intrinsic sizing. The webbrowser lib does not. > I'm still confused as to why the external interfaces require specific support > to handle intrinsic resizing. That's why I thought this was a pain and wondered if we could hide this inside webbrowser and have it just call out to nsIEmbeddingSiteWindow to do the actual sizing once it had been determined. > Is it as simple as finding the inner DOM window and calling |SizeToContent|? Almost. Another thing to watch for is that you may get a call to SetVisibility() before the load happens and you have a chance to size to content. In this case your impl of SetVisibiity should not immediately make the window visible but set a flag which says it should be made visible as soon as you can determine the size.
Comment 5•23 years ago
|
||
I'll do some playing once my build finishes and see if I can figure out a way to work around this for 0.8.1 and something that might fit into the webbrowser lib.
Comment 6•23 years ago
|
||
Thanks for the tips ccarlen. I've implemented what you described in my new embedding widget and it works very well. I'd like to turn the new embedding widget on for 0.8.1 as I've described in bug 72224. Can people from here review? They are most qualified.
Comment 9•23 years ago
|
||
How's the progress on this one? I hope to see it by 0.9.
Comment 10•23 years ago
|
||
It works in the gtk embedding widget. I don't know about the other implementations.
Assignee | ||
Comment 11•23 years ago
|
||
Assignee | ||
Comment 12•23 years ago
|
||
this should do the trick, if someone could verify i found all the code that was using the old method that would be great
Status: NEW → ASSIGNED
Comment 13•23 years ago
|
||
Here's another: http://lxr.mozilla.org/seamonkey/source/embedding/browser/powerplant/source/CBrowserWindow.cp#559.
Comment 14•23 years ago
|
||
And: http://lxr.mozilla.org/seamonkey/source/embedding/browser/gtk/src/EmbedPrivate.cpp#488
Comment 15•23 years ago
|
||
Lots of things here: http://lxr.mozilla.org/seamonkey/ident?i=SizeToContent
Assignee | ||
Comment 16•23 years ago
|
||
ok, attaching a new patch which fixes (i hope ;-) all the issues mentioned.
Comment 17•23 years ago
|
||
Cc'ing jst to bless the DOM IDL change in the patch. /be
Assignee | ||
Updated•23 years ago
|
Whiteboard: estimated landing on wednesday april 18th
Comment 18•23 years ago
|
||
DOM changes look fine to me, please note that binary compatibility will be broken for code that uses nsIDOMWindowInternal since this method is moved, but if that's ok with you it's fine with me in this case. r/sr/a=jst for the IDL changes (and r=jst for the other change), I'm making the same interface change on the XPCDOM branch right now.
Updated•23 years ago
|
Whiteboard: estimated landing on wednesday april 18th → Wanted for 0.9, ETA 4/18
Comment 19•23 years ago
|
||
Please update ETA, current status.
Comment 20•23 years ago
|
||
we seem to have everything ready. when could this be checked-in?
Updated•23 years ago
|
Whiteboard: Wanted for 0.9, ETA 4/18 → Wanted for 0.9, -have patch, and approvals. ready for checkin
Assignee | ||
Comment 21•23 years ago
|
||
everything is not ready, i haven't tested (waiting for mozilla to build right now to test) all the code path changes that i had to make to the code that was using the old interface. i also don't have an sr yet... hopefully i will have both before i leave today
Assignee | ||
Updated•23 years ago
|
Whiteboard: Wanted for 0.9, -have patch, and approvals. ready for checkin → Wanted for 0.9, need more testing and an sr, should be ready soon
Updated•23 years ago
|
Whiteboard: Wanted for 0.9, need more testing and an sr, should be ready soon → need4emb-0.9, need more testing and an sr, should be ready soon
Comment 22•23 years ago
|
||
it would also be good to test this on mfcEmbed. we could really use this in 0.9.
Assignee | ||
Comment 23•23 years ago
|
||
afaik mfcEmbed doesn't currently have any support for intrinsic sizing, it would be great to add it but i don't know any mfc so someone else might have to do it ;-) anyway modified patch (with a few more instances of size_to_content) should be up in a few minutes, i am sorting through some stuff right now
Comment 24•23 years ago
|
||
Patch covering all SizeToContent calls coming up soon? /be
Assignee | ||
Updated•23 years ago
|
Whiteboard: need4emb-0.9, need more testing and an sr, should be ready soon → need4emb-0.9, should land either today (04/25) or tomorrow (04/26)
Assignee | ||
Comment 25•23 years ago
|
||
currently diffing, should post new patch rsn
Assignee | ||
Comment 26•23 years ago
|
||
Comment 27•23 years ago
|
||
r/sr=brendan@mozilla.org /be
Are we going to regress this when the xpcdom branch lands? Arik, could you whip up a fix for that Brave New World as well?
Assignee | ||
Comment 29•23 years ago
|
||
my understanding is that jst has already fixed this on the xpcdom branch and we should be fine... if i'm wrong i could do a fix ;-)
Assignee | ||
Comment 30•23 years ago
|
||
i guess winEmbed added this feature today, will add new patch with patch for winEmbed
Comment 31•23 years ago
|
||
One comment, from the "in my day, we had to walk to school in the snow, in our underwear, uphill both ways" perspective: If you can hunt down the equivalent methods in our other various and sundry testbeds which should or do call SizeToContent(), and patch those calls, that would be good. You want to make sure you get at least winEmbed, mfcEmbed, gtkEmbed, gtkmozembed and powerplant, if appropriate for each. Alternatively, make sure the API is well-documented -- this seems to be the exception rather than the norm, though. See danm if you feel like whining ;) Besides that, though, r=dr. If you *really* wanted, you could check in as the patch stands, but you'd probably rather your work be visible to those who want to use it...
Assignee | ||
Comment 32•23 years ago
|
||
Comment 33•23 years ago
|
||
I already took care of this on the XPCDOM branch...
Comment 34•23 years ago
|
||
a=blizzard for 0.9 Let's get it in.
Assignee | ||
Updated•23 years ago
|
Whiteboard: need4emb-0.9, should land either today (04/25) or tomorrow (04/26) → need4emb-0.9, should land in a few minutes
Assignee | ||
Updated•23 years ago
|
Whiteboard: need4emb-0.9, should land in a few minutes → landed on 0.9 branch, still needs to be added to trunk, will do tomorrow (04/27)
Assignee | ||
Comment 35•23 years ago
|
||
landed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: landed on 0.9 branch, still needs to be added to trunk, will do tomorrow (04/27)
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•