Closed
Bug 67843
Opened 24 years ago
Closed 24 years ago
crash when opening chrome windows and dialogs - DOM global window impl problems
Categories
(Core Graveyard :: Embedding: GTK Widget, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: blizzard, Assigned: blizzard)
Details
Attachments
(2 files)
3.72 KB,
patch
|
Details | Diff | Splinter Review | |
1.16 KB,
patch
|
Details | Diff | Splinter Review |
Right now if you fire up TestGtkEmbed and go to a non-existant site it should pop up a dialog saying that the site couldn't be found. Right now it's just hanging. The problem is in GlobalWindowImpl::SizeOpenedDocShellItem() calling GlobalWindowImpl::GetTreeOwner(nsIBaseWindow **aTreeOwner). The assumption that the tree owner also implements nsIBaseWindow is a bad one. In my case I'm implementing the tree owner myself but I'm not implementing nsIBaseWindow - I'm implementing nsIWebBrowserSiteWindow since it's the site window, not the internal window. And, I can't implement that interface on my object and forward the methods since the names conflict between nsIWebBrowserSiteWindow and nsIBaseWindow. OK, any advice? This is something that I need to fix before 0.8 goes out the door.
Assignee | ||
Comment 1•24 years ago
|
||
Assignee | ||
Comment 2•24 years ago
|
||
This patch lets me implement the rest of nIPrompt since the method signatures for nsIWebBrowserSiteWindow just happen to be the same. It's a hack but it's needed for 0.8 so it will work.
Comment 3•24 years ago
|
||
Comment 4•24 years ago
|
||
Dan, what do you think about the last patch in this bug?
Assignee | ||
Comment 5•24 years ago
|
||
I want to try and get this in for 0.8. Dan, if you can look at this. If you don't have time or you aren't sure I can check in my work around into 0.8. It's ugly but safe.
I'm new to this stuff and I only have like a 70% confidence in what I'm about to say. And my tree is horked right now so I can't doublecheck, *and* I'm more familiar with the Windows embedding test apps. But... I believe the winEmbed test app doesn't choke on this because its mDocShell is implemented by embedding/browser/webBrowser/nsDocShellTreeOwner, which is also an nsIBaseWindow. nsDocShellTreeOwner (that is, the implementation of nsIDocShellTreeOwner) is a Mozilla-supplied thing to be linked into an embedding app, while nsIWebBrowserChrome is a public API, its implementation supplied by the embedding app. In the embedding API review meetings I've been attending, they've been careful to draw that distinction. And I've been checking in code to help define it. I gather that GtkMozEmbedChrome is the top-level window; the thing provided by the embedding app. It should probably be different from the object that implements nsIDocShellTreeOwner; the thing provided browser/webBrowser. So like I said I'm not completely sure, but I think long-term you want to break GtkMozEmbedChrome up. Short term, I think your (Christopher's) patch is more correct than Johnny's.
Comment 7•24 years ago
|
||
r=jst for blizzard's patch.
I love null-safety. sr=shaver
Check me out, reviewing the wrong patch (which is, from the sr= perspective, also quite reasonable). Chris' patch also passes muster. sr=shaver.
Assignee | ||
Comment 10•24 years ago
|
||
Checked in. Thanks, guys.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•