109309 - Move post-loading URL fixup into nsDefaultURIFixup.cpp

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Adam Lock]

nsWebShell::EndPageLoad has some hardcoded stuff to do keyword server lookups
and tack on a www. and a .com to retry and reload a URL that resulted in a
lookup failure the first time around. This should all be moved into the fixup
service.

The fixup service should probably be moved out of docshell and into
mozilla/embedding/components at the same time.

Comment 1

[Adam Lock]

Attachment: Patch moves fixup out of nsWebShell

Rick could you look over this code to see this is the way you'd like to see it
happen?

Basically, I've added a new FIXUP_FLAGS_MAKE_ALTERNATE_URI flag to nsIURIFixup
which tells createFixupURI to turn foo into www.foo.com. The
nsWebShell::EndPageLoad method now calls the fixup object.

I've also tightened up the security aspect a little bit. Only http://foo will
be expanded, not https://foo. Urls with a username or password are ignored too.


You will see one slight inefficiency in the code. I compare URIs in a few
places, but I discovered a bug in the impl nsIURI::Equals which meant it didn't
work, so instead I've opted to do a straight string compare on each URI's spec.
I emailed Darin Fisher about the problem with Equals so if that gets fixed,
maybe my code can use the method. A new bug should probably be raised on moving
the fixup object out of docshell.

Comment 2

[Adam Lock]

Attachment: New patch does equality testing properly

nsIURI::Equals is working again so this patch does URI comparison the proper
way.

Rick can you review this?

Comment 3

[rpotts (gone)]

Comment on attachment 61488 [details] [diff] [review]
New patch does equality testing properly

looks good to me. r/sr=[e-mail]

Comment 4

[Conrad Carlen (not reading bugmail)]

Comment on attachment 61488 [details] [diff] [review]
New patch does equality testing properly

Looks good. r=ccarlen.

One suggestion though:

>+    // Count the dots
>+    PRInt32 numDots = 0;
>+    PRInt32 idx = 0;
>+    do {
>+        idx = oldHost.FindChar('.', idx);
>+        if (idx != -1) {
>+            numDots++;
>+        }
>+    } while (idx != -1);
>+

Instead of the overhead of calling FindChar() in the loop, you could just
iterate over the characters in the string.
nsReadingIterator<char> start, end;
oldHost.BeginReading(start);
oldHost.EndReading(end);
while (start != end) {
...
}

Comment 5

[Adam Lock]

Thanks Conrad & Rick. Fix is checked in with Conrad's suggestion incorporated.

Comment 7

[benc]

V/fixed.

I looked at lxr for the edited files and eyeball the change.

I've read through the newer fixed version of the code in the past, and it seemed to behave as desired.