236608 - Domain Guessing: URL is not updated when guessing loads www.hostname.com

Status: VERIFIED FIXED

(SeaMonkey :: Location Bar, defect)

Description

[Erich 'Ricky' Iseli]

Steps to reproduce:
1. type münster.de in location bar and hit enter
2. let the page load completely


Result:
check the location bar: the address displayed is still "münster.de"

Expected result:
the address displayed should be http://www.münster.de/

Even worse:
3. Click on any link on this page, for example
http://publikom.muenster.de/stadt/index.html

Result:
The address still reads münster.de

Expected result:
the address displayed should be http://publikom.muenster.de/stadt/index.html

Workaround: hit Ctrl-Shift-R in order to have everything work as intended.

Comment 1

[Jo Hermans]

Reporter, I get the same behaviour with muenster.de. This isn't IDN related.

Comment 2

[Erich 'Ricky' Iseli]

(In reply to comment #1)
> Reporter, I get the same behaviour with muenster.de. This isn't IDN related.

Yeah, you're right. That's weird though. I've made some tests with different
Umlaut and non-Umlaut domain names:

- bücher.ch -> http://www.bücher.ch/
- buecher.ch -> buecher.ch
- müller.ch -> (redirected) http://müller.ch/default.asp?Lan=de&
- switch.ch -> http://www.switch.ch/

So the question would be: why does münster.de, muenster.de and buecher.ch stay
as is, while others have the expected behavior. Removing the "IDN" part of the
description.

Comment 3

[Simon Montagu :smontagu]

If the bug isn't IDN-specific, it probably belongs in another component.

Comment 4

[David Balažic]

Same happens with "sophos.si" ( except that after clicking a link the address is
displayed correctly ) , mozilla 1.7b on MS Windows 2000 pro-SP4.

The difference with switch.ch is that http://switch.ch exists and is a redirect
(301 Moved Permanently) to http://www.switch.ch/ , while sophos.si (and
muenster.de) is not registered in DNS and mozilla tries www.sophos.si instead
when it fails.

Comment 5

[Wolf [:wolf]]

I'm seeing this behavor also on Windows with sophos.si.. It appears to be an
issue with Domain Guessing enabled.. and the address bar is not being updated to
reflect the change in domain..

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040316


This is Mozilla Suite specific though.. WFM on Firefox 0.8..

Updating OS to All.. 

Comment 6

[benc]

CONFIRMED:
Mozilla 1.6f, Mac OS X. Netscape 7.1 (which was Mozilla 1.4 based).

Does anyone know how far back this was broken? I don't use this feature myself,
it is turned off in most of my builds. I also was not test tester of this
feature, and from looking at my results, I may not have had a test case that
would have shown this regression.

This also might be related to newer profiles. I am actually working on a lot of
stuff right now and can't close my session.

Comment 7

[benc]

requesting 1.7 fix. per asa, this is bad.

Comment 8

[benc]

*** Bug 209713 has been marked as a duplicate of this bug. ***

Comment 9

[Joseph Delaney]

(In reply to comment #6)
> Does anyone know how far back this was broken? 

It stopped working correctly sometime in early June 2003. This bug was present
in 1.4rc2, but (if I remember correctly) 1.4rc1 worked properly.

Comment 10

[Mike Kaply [:mkaply]]

This definitely broke between 1.4rc1 and 1.4rc2.

I'm doing some date pulls to try to narrow it down.

We might have some 1.4 builds on OS/2 from these dates. I'm checking.

Comment 11

[Mike Kaply [:mkaply]]

This broke between June 11 and June 12 2003

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_4_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=06%2F11%2F2003+8%3A00&maxdate=06%2F12%2F2003+8%3A00&cvsroot=%2Fcvsroot

Culprit appears to be bug 104778

I'm backing that out of my 1.4 branch to see.

Comment 12

[Mike Kaply [:mkaply]]

Specifically, it is this change to tabbrowser.xml:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/xpfe/global/resources/content/bindings&command=DIFF_FRAMESET&file=tabbrowser.xml&rev1=1.72.4.1&rev2=1.72.4.2&root=/cvsroot

Comment 13

[Darin Fisher]

So, I just tested a trunk debug linux build of Firefox, and I cannot reproduce
this bug with that build.  However, using the latest 1.7 build, the problem does
occur.  So, it seems that we have fixed this on the trunk.

Comment 14

[Darin Fisher]

Whoops.  It seems that I spoke too soon.  I went and tried a today's trunk build
of Seamonkey and found that it too exhibits this bug.  Perhaps the tabbrowser
files were only changed for Seamonkey and not Firefox.  I confess that I don't
know the tabbrowser implementation well at all.  Time to update my Seamonkey
debug build!

Comment 15

[Darin Fisher]

nsBrowserStatusFilter::OnLocationChange is definitely receiving a notification
that the URI changed to http://www.münster.de/, so that confirms that the core
code is working correctly.  Now, to take a closer look at tabbrowser.

Comment 16

[Darin Fisher]

ooh.. interesting.  i also see the nsBrowserStatusHandler.onLocationChange
method being called.

Comment 17

[Darin Fisher]

and then in nsBrowserStatusHandler.onLocationChange, i get this JS exception:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "userTypedvalue is not defined" {file:
"chrome://navigator/content/nsBrowserStatusHandler.js" line: 322}]' when calling
method: [nsIWebProgressListener::onLocationChange]"  nsresult: "0x80570021
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"  data: yes]
************************************************************

investigating further...

Comment 18

[Darin Fisher]

Ignore that exception.  It was caused by my own hacking, but it does reveal the
fact that in this code,

http://lxr.mozilla.org/mozilla/source/xpfe/browser/resources/content/nsBrowserStatusHandler.js#305

we are taking the else branch which does |this.urlBar.value = userTypedValue;|.

so, it seems that something is causing us to prefer the user typed value over
the new location.  just above this code block is the comment:

  // Update urlbar only if a new page was loaded on the primary content area
  // Do not update urlbar if there was a subframe navigation

interestingly enough, it turns out that the entire visible portion of the page
is contained in subframes.  the toplevel URL results in this frameset source:

<html>
<head>
<title>das publikom - Willkommen</title>
</head>
<frameset rows="42,*" frameborder="no" border=0>
  <frame name="pubmenu" src="http://publikom.muenster.de/welcome/menu.html"
                marginwidth="0" marginheight="0"
                scrolling="no" frameborder="0"
                border="0" noresize>
  <frame name="pubbody" src="http://publikom.muenster.de/welcome/content.html"
                marginwidth="0" marginheight="0"
                scrolling="auto" frameborder="0">
</frameset>
</html>

however, this doesn't excuse the bug because this frameset can only be gotten by
loading www.münster.de (IDN), www.xn--mnster-3ya.de (ACE), or www.muenster.de
(ASCII-ized German).

it seems that the logic for suppressing location changes for subframes is
affecting location changes for the frameset.  i think at this point i should
create a reduced testcase.

Comment 19

[Darin Fisher]

jag, neil: do either of you have any ideas on this bug?

Comment 20

[Darin Fisher]

more information:

i see endDocumentLoad for the failed URL happening after the startDocumentLoad
for the fixed-up URL !!

that seems to screw up the userTypedClear logic.

here's the output from my logging:

startDocumentLoad [http://xn--mnster-3ya.de/] userTypedClear <= true
startDocumentLoad [http://www.xn--mnster-3ya.de/] userTypedClear <= true
endDocumentLoad [http://xn--mnster-3ya.de/,2152398878] userTypedClear <= false
Error loading URL http://www.münster.de/ : 804b001e
nsBrowserStatusHandler.onLocationChange [http://www.münster.de/]
setting urlbar to userTypedValue [http://münster.de/]

looking at the code in nsBrowserStatusHandler.onLocationChange, it is clear that
if userTypedClear is true then the URL bar will be updated to the location
passed to onLocationChange.  otherwise, we require that the URL bar keep showing
the userTypedValue.

so i think the solution here is to make sure the endDocumentLoad for the failed
URL load happens before startDocumentLoad for the fixed-up URL.

Comment 21

[[e-mail]]

Hmm... is it actually necessary to reset userTypedClear in endDocumentLoad?
(Sorry that I can't think straight, but it's 00:41 local time...)

Comment 22

[Darin Fisher]

I think the correct solution may be to make this code:

http://lxr.mozilla.org/mozilla/source/docshell/base/nsWebShell.cpp#872

happen after nsWebShell::EndPageLoad completes.  This is necessary because
LoadURI calls nsIChannel::AsyncOpen, which calls nsILoadGroup::AddRequest, which
calls the loadgroup's groupObserver, which is the docloader, which then calls
OnStateChange(STATE_START) on all the nsIWebProgressListeners.  For this to all
happen while we are dispatching OnStateChange(STATE_STOP) notifications seems
really wrong.

...

So, I tried "proxying" the call to LoadURI to see if that fixes the problem, and
indeed it does.  Of course, proxying adds complexity here because we have to
worry that the docshell might have decided to go off and load something else
while we were waiting for our delayed LoadURI to run.

Also, what if someone else calls LoadURI while we are inside
OnStateChange(STATE_STOP)?  Wouldn't that lead to the very same problem?  Should
LoadURI always happen asynchronously?  I seem to recall some cases where the
Javascript protocol handler would call LoadURI recursively, the Javascript
protocol handler is notoriously fragile :-(

I'm not sure that we can get away with always making LoadURI happen
asynchronously.  That's a big scary change.

Comment 23

[Darin Fisher]

(In reply to comment #21)
> Hmm... is it actually necessary to reset userTypedClear in endDocumentLoad?
> (Sorry that I can't think straight, but it's 00:41 local time...)

Maybe it is, maybe it isn't.  I don't really understand all the ways in which it
is used.  It seems logical that the author of that code probably assumed that
he'd see STOP notifications before START notifications.

The other way to look at this is that we could try to make
nsBrowserStatusHandler.js smarter.  We could maybe turn userTypedClear into a
simple integer counter that we increment in startDocumentLoad and decrement in
endDocumentLoad.  Then maybe we'd also do the right thing.

I'll test that as well.

Comment 24

[Darin Fisher]

yeah, turning userTypedClear into a counter seems to also work.  i'll attach
both patches for review.

Comment 25

[Darin Fisher]

Attachment: v1 patch : make URL fixup call LoadURI via a PLEvent

Comment 26

[Darin Fisher]

Attachment: v2 patch : turn browser.userTypedClear into a counter

Comment 27

[Darin Fisher]

both of these patches fixes this bug.  the v2 patch is probably the safer of the
two.

i'd really like someone who knows tabbrowser.xml and nsBrowserStatusHandler.js
to comment on this stuff.

it's interesting that the problem doesn't show up in Firefox, but that just
might result from the nsIWebProgressListener's being hooked up in a different
order :-/

Comment 28

[Boris Zbarsky [:bzbarsky]]

I'm loath to make changes to the docshell code until we have clear documentation
on how progress listeners should work; at that point we should make the code
comply with the documentation.

Comment 29

[Darin Fisher]

boris: you're starting to sound like a broken record! ;-)

Comment 30

[[e-mail]]

Comment on attachment 148255 [details] [diff] [review]
v2 patch : turn browser.userTypedClear into a counter

Sorry for the delay, I couldn't get the patch to apply, there was too much
context to cleanly merge with my other patches.

Comment 31

[Asa Dotzler [:asa]]

RC2 is fast approaching. Jag, can you help us with a super-review on this today?

Comment 32

[benc]

Darin: are you turning off Internet Keywords in Firefox? That will trap almost
all test cases for domain guessing.

Comment 33

[Darin Fisher]

> Darin: are you turning off Internet Keywords in Firefox? That will trap almost
> all test cases for domain guessing.

No, I'm not doing anything like that.  This patch fixes a core tabbrowser bug
that could be triggered by anything that causes the browser to load a new URL
while it is in the process of stopping a previous load.  In this case, the
result of the bug is that the URL bar does not update properly.  That problem
could also theoretically happen if I wrote an extension that kicked off a load
whenever it found that a URL load failed (via nsIWebProgressListener).

Comment 34

[Daniel Veditz [:dveditz]]

Comment on attachment 148255 [details] [diff] [review]
v2 patch : turn browser.userTypedClear into a counter

sr=dveditz

Comment 35

[chris hofmann]

Comment on attachment 148255 [details] [diff] [review]
v2 patch : turn browser.userTypedClear into a counter

a=chofmann for 1.7rc2

Comment 36

[Darin Fisher]

fixed-on-trunk

Comment 37

[Darin Fisher]

fixed1.7

Comment 38

[David Balažic]

I still see this in FIreFox 1.0 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.7.5) Gecko/20041107 Firefox/1.0), when entering "sophos.si".

Should this bug be reopened or a new one for FireFox opened ?

Comment 39

[Joseph Delaney]

It's already been opened at Bug 264610

Comment 40

[benc]

V/fixed.
Mozilla 1.8a4 & 1.7.3 on Mac OS.

Please reopen if it is busted in mozila-only, on any plat.

I'm also leaving this in core b/c I'm assuming this is still broke in firefox
b/c of the aviary branch, not b/c they have their own feature fork of location bar.

Comment 41

[Daniel Veditz [:dveditz]]

*** Bug 219364 has been marked as a duplicate of this bug. ***