351777 - Domain fix up/guessing is broken.

Status: VERIFIED FIXED

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

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: 2006-09-07-10, Windows XP SeaMonkey trunk.

Summary: Domain guessing/fix up is broken.

In SeaMonkey trunk, if you type "domain" into the URL bar, we no longer fix up the "domain" entry into www.domain.com.

This seems to be a regression from bug 323798.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Actually, this is a regresssion from bug 349419.

The basic issue we run into in this bug is the following code in nsWebShell::EndPageLoad:

1078         // Skip fixup for anything except a normal document load operation on
1079         // the topframe.
1081         if (mLoadType != LOAD_NORMAL || !isTopFrame)
1082         {
1083           doCreateAlternate = PR_FALSE;
1084         }

In this case, of course, mLoadType is LOAD_NORMAL_FIXUP.

In my opinion, we should just back out the patch from bug 349419 and instead change LoadURI (the string version) to not propagate the fixup flag any deeper into docshell (that is, mask it off the incoming flags after we try fixup but before we create the loadinfo).  This used to sorta happen, with some asserts, because ConvertLoadTypeToDocShellLoadInfo dedaults to nsIDocShellLoadInfo::loadNormal for unknown load types.  But that changed with the patch for bug 349419.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, the LOAD_NORMAL_FIXUP thing also breaks bfcache for loads done from the URL bar, since bfcache only happens for certain load types and LOAD_NORMAL_FIXUP is not one of them.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch per comment #1

This seems to fix this bug without regressing bug 349419, though I'd like to verify before asking for review.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 239499 [details] [diff] [review]
patch per comment #1

er, this patch is wrong.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch per comment #1

I've tested this in a debug Firefox build, with both values of keyword.enabled.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 239502 [details] [diff] [review]
patch per comment #1

Nix the whitespace changes and sr=bzbarsky

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 239502 [details] [diff] [review]
patch per comment #1

+    aLoadFlags = aLoadFlags & ~LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;

I'd write this as:
  aLoadFlags &= ~LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated to review comments

Comment 9

[:Gavin Sharp [email: [e-mail]]]

mozilla/docshell/base/nsDocShell.cpp 	1.816
mozilla/docshell/base/nsDocShellLoadTypes.h 	1.4
mozilla/docshell/base/nsIDocShellLoadInfo.idl 	1.23

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using trunk build 2006-09-24-09 of SeaMonkey under Windows XP.