67084 - Need to refactor state change notifications in docShell

Status: RESOLVED INCOMPLETE

(Core :: DOM: Navigation, defect)

Description

[Simon Fraser [no longer active]]

The stuff that happens in nsDocShell::OnStateChange() is gnarly and fragile, and 
not factored in a way that makes it possible for derived classes to override the 
default behaviour.

For editor embedding, I'm going to be makeing a subclass of nsDocShell (actually 
nsWebShell currently), which needs to hook in to document loads. By factoring out 
the code in OnStateChange into discrete, overridable methods, I am able to do 
this in a clean way. Patch coming...

Comment 1

[Simon Fraser [no longer active]]

Attachment: Docshell patch to refactor state changes

Comment 2

[Simon Fraser [no longer active]]

With this patch, I've tried to move the recently added cursor and redirection 
stuff into separate methods. One thing that I'm not quite clear on is whether the 
NotifyingCurrentDocument() test is appropriate for every 'STATE_IS_DOCUMENT' type 
state change. Please review.

Comment 3

[Adam Lock]

Simon, the patch looks fine though I'm a bit concerned that you want to subclass 
nsDocShell. Is this something we want to do considering how hateful webshell is? 
If may be more desirable to use some kind of XPCOM notification mechanism 
instead.

Comment 4

[Simon Fraser [no longer active]]

I've reimplemented the editor stuff to work without any subclassing. I no longer 
require this state change notification refactoring, but it still seems a good 
thing for maintainability reasons.

Comment 5

[Judson Valeski]

agreed. nominating mozilla0.9.1

Comment 6

[Adam Lock]

Future

Comment 7

[basic]

Changing platform

Comment 8

[Edgar Chen [:edgar]]

The codebase have changed a lot since then, so close this as INCOMPLETE.