Closed Bug 621274 Opened 13 years ago Closed 13 years ago

Location Bar remembers invalid (Server not found) URLs


(Firefox :: Address Bar, defect)

Not set



Firefox 4.0b10
Tracking Status
blocking2.0 --- final+


(Reporter: c.ascheberg, Assigned: mak)




(Keywords: regression, Whiteboard: [softblocker][fixed-in-places])


(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows NT 6.0; rv:2.0b9pre) Gecko/20101223 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Windows NT 6.0; rv:2.0b9pre) Gecko/20101223 Firefox/4.0b9pre

See STR below.
This is a regression:
win32 2010-12-15-03-mozilla-central f11f7ed625ba
win32 2010-12-16-03-mozilla-central a5413c3c1013

Reproducible: Always

Steps to Reproduce:
1. visit for example http://www, (note the comma instead of dot)
2. firefox will redirect to http://www.www, and show Server not found error page
3. type google into location bar
Actual Results:  
http://www.www, will show up as a result.

Expected Results:  
I think this should not show up.
I wonder why firefox is actually trying to fix the URL like this instead of showing a more specific error page.
Keywords: regression
Version: unspecified → Trunk

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101215
Firefox/4.0b9pre ID:20101215174043
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101215
Firefox/4.0b9pre ID:20101215204205
Linux too
Mozilla/5.0 (X11; Linux i686; rv:2.0b9pre) Gecko/20101223 Firefox/4.0b9pre ID:20101223030401
OS: Windows Vista → All
Please ignore comment #3,
I can not reproduce on Linux
OS: All → Windows Vista
In local build
Build from af7e6f7cbf16 : fails
Build from 2a7a3641d67d : fails
Build from 6617dcb0d48a : fails
Build from d65c65ef13a4 : works
Build from 20311c05703e : works
Build from b198eca4d183 : works
Build from 4626f19fa27e : works

So, this is regressen by Bug 599973 - Don't use steps for async favicons
Blocks: 599973
blocking2.0: --- → ?
Ever confirmed: true
And also
Build from 999385f355a3 : fails
blocking2.0: ? → final+
I have to check what we were doing before, but most likely part of the problem is in  "INSERT INTO moz_places (url, rev_host, favicon_id) ", where we don't set hidden = 1, I thought that one was the default value but looks like we default to 0 :(
Still, we should not try to add invalid pages since I'd guess we fail to gather a icon from network (onStopRequest) and we use the error icon.
Assignee: nobody → mak77
Blocks: 621273
Flags: in-testsuite?
OS: Windows Vista → All
Hardware: x86 → All
Attached patch patch v1.0 (obsolete) — Splinter Review
Apart fixing icons service to avoid storing error icons and marking pages as hidden by default, I changed feedWriter use of history and favicons services, because it was verbose and not efficient.
The new code is simpler, kills a useless history observer and a bunch of main-thread IO. It still passes through favicon service and saves icons for handlers, we could maybe avoid it, but first of all I want to avoid privacy/security regressions (I practically replicated code path as it was) and second it just happens in feed previews so it doesn't matter much globally.
This components needs more cleanup but post FX4.
Attachment #503024 - Flags: review?(sdwilsh)
Whiteboard: [softblocker]
Comment on attachment 503024 [details] [diff] [review]
patch v1.0

>+++ b/toolkit/components/places/tests/head_common.js
>+ * Returns the hidden status of a url.
>+ *
>+ * @param  aURI
>+ *         The URI or spec to get hidden for.
nit: two spaces after @param; should only be one

>+ * @return the hidden value.
nit: @return true if the url is hidden, false otherwise.

>+++ b/toolkit/components/places/tests/unit/test_doSetAndLoadFaviconForPage_failures.js
>+    desc: "test setAndLoadFaviconForPage with error icon",
>+    pageURI: uri(""),
>+    go: function go4() {
>+      PlacesUtils.favicons.setAndLoadFaviconForPage(
>+        this.pageURI, NetUtil.newURI(FAVICON_ERRORPAGE_URL), true
>+      );
>+    },
You are using uri() and NetUtil.newURI.  I'd prefer the latter.

>   { // This is a valid icon set test, that will cause the closing notification.
>     desc: "test setAndLoadFaviconForPage for valid history uri",
>-    pageURI: uri(""),
>-    go: function go4() {
>+    pageURI: uri(""),

>     // Ensure we have been called by the last test.
>-    do_check_true(pageURI.equals(uri("")));
>+    do_check_true(pageURI.equals(uri("")));

Attachment #503024 - Flags: review?(sdwilsh) → review+
Attached patch patch v1.1Splinter Review
Attachment #503024 - Attachment is obsolete: true
Flags: in-testsuite? → in-testsuite+
Whiteboard: [softblocker] → [softblocker][fixed-in-places]
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
Blocks: 627416
No longer blocks: FF2SM
Verified with Firefox 4.0b10.

Note: Added an extra step to the original STR...

Original STR:
1. Visit www,
2. Firefox loads http://www.www, and shows Server not
found error page
3. Type "google" into the location bar

1. Visit www,
2. Firefox loads http://www.www, and shows Server not
found error page
3. Click HOME
4. Type "google" into the location bar

Without the addition of clicking the HOME button, this will load a switch-to-tab result for http://www.www, because it is still an active tab.  I believe this is expected.
Depends on: 1392221
You need to log in before you can comment on or make changes to this bug.