Closed Bug 113970 Opened 18 years ago Closed 18 years ago

Clean up mozilla/webshell

Categories

(Core :: DOM: Navigation, defect)

x86
Windows 2000
defect
Not set

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: stdowa+bugzilla, Assigned: adamlock)

Details

Attachments

(1 file)

Is this file still needed? It only contains:

#include "nsIWebShell.h"

void
XXXNeverCalled()
	{
	}
Large swathes of mozilla/webshell could be removed I suspect.

1. mozilla/webshell/embed
   I have no idea wtf ngprefs is but it looks like it's never been touched since
   it was added 3 years ago
2. mozilla/webshell/src
   All webshell source got moved to to mozilla/docshell
3. mozilla/webshell/tests
   I *really* wish I could kill viewer. I suspect the layout people may still
   use it, though they should really be testing in mfcEmbed or Mozilla
   these days.

So all we'd be left with is mozilla/webshell/public. This contains the
nsIWebShell* interfaces and could either stay where it is or move over to
docshell (which could use a mozilla/docshell/public directory)
Summary: cvs remove webshell/src/dlldeps.cpp? → Clean up mozilla/webshell
Target Milestone: --- → Future
Also, the following methods on nsIWebShell appear to be killable:

FindNext
GetRootWebShell
GetReferrer
GetTopLevelWindow
GetHistoryLength
GetHistoryIndex (with some work to viewer.exe)
GetURL (ditto)
SetURL

And their impls in nsWebShell plus these methods too

GetParent
SetParent
GetRootWebShellEvenIfChrome

That should leave some nsIWebShell gristle to deal with nsIWebShellContainer
that's about all
Attached patch Patch Splinter Review
This patch brutally chops out 200+ lines of dead webshell crap, fixes a few
oddities in docshell caused by removal of nsWebShell overides (e.g.
Get/SetDimensions) and makes nsDocShell::SetCurrentURI a method on nsIDocShell
to replace nsIWebShell::SetURL which has been removed.

Patch also modifies editor slightly to call nsIDocShell::SetCurrentURI instead
of SetURL and ensures viewer.exe still work.

Now nsIWebShell is reduced to 3 methods - SetContainer, GetContainer and
GetDocumentLoader.

Reviews please?
sweet! all those methods just go away (no callsites in mozilla or commercial?)?
If so, r=valeski.
I'll build commercial to make sure but there doesn't appear to be anywhere it
calls webshell methods.

Rick do you want to sr this one?
Verifying that commercial build is happy with these changes.
Comment on attachment 62388 [details] [diff] [review]
Patch 

ya!!  Adam, thanks for cleaning up all of this nasty cruft!!
Attachment #62388 - Flags: superreview+
Fix is checked in
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Actually, I'll reopen this bug until I've had time to delete all the unused
files still festering in mozilla/webshell.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I'm just wondering about the Qt code you removed... what's 'dead' about it? Just
curious.
mozilla/webshell/Makefile.in didn't even reference mozilla/webshell/embed so the
QTstuff has not been built on Unix for some time. Besides which the code is
prehistoric and wouldn't even work without being rewritten.

So webshell is reduced to a gnarly stump of public headers can be transplanted
to docshell at some point and the viewer test app. I wish viewer would die. I
wonder if anyone still needs it.

Marking this bug FIXED.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Layout people still need viewer.  To get rid of it we'd need some other app that
had the debugging capabilities that are in viewer's menus, and we'd need to
rewrite the layout regression test harness to use some other app.  That's
certainly doable, but it's a bit of work and nobody's signed up to do it.
Raised bug 122829 to move viewer into layout.
You need to log in before you can comment on or make changes to this bug.