nsXULWindow::GetContentShellById was hidden by nsWebShellWindow::GetContentShellById

RESOLVED FIXED

Status

()

Core
Document Navigation
RESOLVED FIXED
17 years ago
17 years ago

People

(Reporter: timeless, Assigned: Adam Lock)

Tracking

Trunk
x86
FreeBSD
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
In file included from /home/timeless/mozilla/xpfe/appshell/src/nsAppShellService.cpp:59:
/home/timeless/mozilla/xpfe/appshell/src/nsXULWindow.h:63: warning: `nsXULWindow::GetContentShellById(const PRUnichar *, nsIDocShellTreeItem **)' was hidden
/home/timeless/mozilla/xpfe/appshell/src/nsWebShellWindow.h:88: warning:   by `nsWebShellWindow::GetContentShellById(const nsString &, nsIWebShell **)'
/home/timeless/mozilla/xpfe/appshell/src/nsAppShellService.cpp: In method `nsresult nsAppShellService::JustUseBrowserSibling(nsIXULWindow *, nsIURI *, int, unsigned int, nsIXULWindow **)':

this is from a 097branch, but the code appears to be the same on trunk.  I'm actually trying to use one of these functions, so it'd be nice if this hiding didn't happen.
(Assignee)

Comment 1

17 years ago
It looks like both nsIXULWindow and nsIWebShellWindow have GetContentShellById
methods.

nsIXULWindow defines it like this:

  /* nsIDocShellTreeItem getContentShellById (in wstring ID); */
  NS_IMETHOD GetContentShellById(const PRUnichar *ID, nsIDocShellTreeItem
**_retval) = 0;

And nsIWebShellWindow (which is not generated from IDL) defines it like this:

NS_IMETHOD GetContentShellById(const nsString& anID, nsIWebShell** aResult) = 0;

Even though one is a takes a ref and the other a pointer and both return
different interfaces, they're probably too close for comfort for the compiler. 

I would recommend the nsWebShellWindow::GetContentShellById version and the
method on nsIWebShellWindow be totally removed. Only nsWebShellWindow even calls
 this method and totally ignores the return value anyway so there's no point in
the override.


(Assignee)

Comment 2

17 years ago
Created attachment 67337 [details] [diff] [review]
Patch removes nsIWebShellWindow::GetContentShellById

Please try out the patch.

It's straightforward enough and it builds but I'll have to do a full build to
verify it functions correctly. I'll do that first thing tomorrow.
(Assignee)

Comment 3

17 years ago
Timeless can you review this patch?

I've built and it appears to work but I have no idea how to test the call to
GetContentShellById in nsWebShellWindow::LoadContentAreas. There should be no
reason for it to fail as old impl did little more than wrap the impl in
nsXULWindow with an extra QI anyway.

nsWebShellWindow is a rat's nest of bad code.
(Reporter)

Updated

17 years ago
Attachment #67337 - Flags: review+

Comment 4

17 years ago
Comment on attachment 67337 [details] [diff] [review]
Patch removes nsIWebShellWindow::GetContentShellById

sr=jag (r=timeless, it seems)
Attachment #67337 - Flags: superreview+
(Assignee)

Comment 5

17 years ago
Fix is in
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

17 years ago
Reopening.

Changes got backed out because of bug 123572. I have no idea what is causing
this - possibly an nsIDocShellTreeItem not implementing nsIWebNavigation but
since I couldn't even get Mozilla to step into this piece of code...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 7

17 years ago
Marking FIXED again. Changes didn't get backed out (so far)
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.