Closed Bug 236608 Opened 20 years ago Closed 20 years ago

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

Categories

(SeaMonkey :: Location Bar, defect)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.7final

People

(Reporter: Erich.Iseli, Assigned: darin.moz)

References

Details

(Keywords: regression, verified1.7)

Attachments

(2 files)

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.
Reporter, I get the same behaviour with muenster.de. This isn't IDN related.
(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.
Summary: http://www. not prepended when entering IDN address, location bar doesn't change any more → http://www. not prepended when entering some address, location bar doesn't change any more
If the bug isn't IDN-specific, it probably belongs in another component.
Assignee: smontagu → darin
Component: Internationalization → Networking
QA Contact: amyy → benc
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.
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.. 
OS: Linux → All
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.
Component: Networking → Location Bar
Keywords: regression
Hardware: PC → All
Summary: http://www. not prepended when entering some address, location bar doesn't change any more → Domain Guessing: URL is not updated when guessing loads www.hostname.com
Version: Other Branch → Trunk
requesting 1.7 fix. per asa, this is bad.
Flags: blocking1.7?
Flags: blocking1.7? → blocking1.7+
*** Bug 209713 has been marked as a duplicate of this bug. ***
(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.
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.
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.7final
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.
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!
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.
ooh.. interesting.  i also see the nsBrowserStatusHandler.onLocationChange
method being called.
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...
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.
jag, neil: do either of you have any ideas on this bug?
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.
Hmm... is it actually necessary to reset userTypedClear in endDocumentLoad?
(Sorry that I can't think straight, but it's 00:41 local time...)
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.
(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.
yeah, turning userTypedClear into a counter seems to also work.  i'll attach
both patches for review.
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 :-/
Attachment #148255 - Flags: superreview?(jag)
Attachment #148255 - Flags: review?(neil.parkwaycc.co.uk)
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.
boris: you're starting to sound like a broken record! ;-)
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.
Attachment #148255 - Flags: review?(neil.parkwaycc.co.uk) → review+
RC2 is fast approaching. Jag, can you help us with a super-review on this today?
Darin: are you turning off Internet Keywords in Firefox? That will trap almost
all test cases for domain guessing.
> 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 on attachment 148255 [details] [diff] [review]
v2 patch : turn browser.userTypedClear into a counter

sr=dveditz
Attachment #148255 - Flags: superreview?(jag)
Attachment #148255 - Flags: superreview+
Attachment #148255 - Flags: approval1.7?
Comment on attachment 148255 [details] [diff] [review]
v2 patch : turn browser.userTypedClear into a counter

a=chofmann for 1.7rc2
Attachment #148255 - Flags: approval1.7? → approval1.7+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
fixed1.7
Keywords: fixed1.7
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 ?
It's already been opened at Bug 264610
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.7verified1.7
*** Bug 219364 has been marked as a duplicate of this bug. ***
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: