51702 - URI fixup & keyword should be moved out of docshell into a service

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Adam Lock]

Docshell & webshell perform URI fixup and keyword detection before starting 
a load. This code should be taken out of docshell and moved into a service. It 
would be desirable for this service to be a Javascript object to allow people to 
customise it's behaviour, e.g. use a different keyword lookup service, fixup 
custom URIs, localized fixup - www.blah.co.jp instead of www.blah.com etc.

Comment 1

[Adam Lock]

Attachment: Docshell with keyword stuff removed

Comment 2

[Adam Lock]

Attachment: New URI fixup interface

Comment 3

[Adam Lock]

Attachment: New fixup object, the default one

Comment 4

[Judson Valeski]

suggestion: rather than having a NS_NewFoo() global function, might I suggest
that you use a default contructor so you can use the component manager for creation?

Comment 5

[Adam Lock]

That's phase 2 :)

I may make the docshell pick up the fixup object from a category and 
additionally, add a property to one of the public interfaces that allows the 
fixup object to be replaced by the client.

Comment 6

[Adam Lock]

Attachment: Latest fixup patch makes the fixup object a global service. Submitting this one for review.

Comment 7

[Adam Lock]

Attachment: Oops, try I'll try that again with a context diff

Comment 8

[Chris Waterson]

Here are my (decidedly uninformed) observations

1. Seems to me that the "URI fixup" object should be a service: my guess is that
it is an application-wide policy, right? (Or, am I wrong? Do we want to vary the
fixup that's performed on a docshell-by-docshell basis?)

2. If you agree that it should be a service, then it seems like we should move
the code that grovels through prefs into the service's implementation. In other
words, why does docshell know about "keywords"? We don't want to change docshell
the next time someone gets a wild hair up their ass about typing random crap
into the URL bar! :-) It'd be nice to just update the fixup policy...

3. If you agree with (1) and (2), then it seems like the fixup API could be a
bit narrower: "nsIURI fixup(in wstring aURIishString)". Or are the fixup flags
still necessary?

Let me know if I'm off base...

Comment 9

[Adam Lock]

Chris, in response to your points:

1. It is a service. The latest patch makes it a global service, requested for 
by each docshell via a contract.
2. Yes, the prefs code could be moved from the docshell side into the service. I 
can try and prepare a patch that does that and see if there are any issues.

Comment 10

[Adam Lock]

Attachment: Revised patch removes the flags parameter and the test for keyword support into the fixup service

Comment 11

[Adam Lock]

For the new patch I've moved the test for keywords support into the fixup
service and removed the flags parameter from the interface. I've also changed
docshell slightly so it calls NS_NewURI if it can't find the fixup service
(because it's not registered, compiled-in or whatever).

Comment 12

[Chris Waterson]

This looks great to me. You can count me as either r= or sr=.

Comment 13

[Adam Lock]

Thanks Chris, fix checked into trunk

Comment 14

[Adam Lock]

Marking fixed