338864 - ctrl+enter doesn't work if the url starts with www

Status: RESOLVED FIXED

(Firefox :: Keyboard Navigation, defect)

Description

[nima]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

the ctrl+enter (and also shift+enter and ctrl+shift+enter) shortkey doesn't work if the url starts with "www" , for example for the www.wwwbazar.com url you can not just type wwwbazar and use ctrl+enter to automatically add "www." and ".com" .

Reproducible: Always

Steps to Reproduce:
1.type wwwbazar
2.use ctrl+enter (or shift+enter or ctrl+shift+enter)
3.

Actual Results:  
the "www." and ".com" will not be added automatically

Expected Results:  
the "www." and ".com" must be added automatically

Comment 1

[Simon Bünzli]

This seems intentional: http://lxr.mozilla.org/mozilla/source/browser/base/content/browser.js#2266 Although testing for /^(www|https?)\b|\/\s*$/i might be more appropriate.

Comment 2

[Simon Bünzli]

Attachment: allow words starting with www to be completed

... and also adding https to the list of non-completable words.

Comment 3

[Ed Lee :Mardak]

Attachment: allow www, fix bitrot, too many other changes, badpatch

I'm probably trying to do too much with one single bugzilla change, but hopefully zeniko can take some suggestions, gavin can provide feedback, and nima can be happy there's activity.

-  var url = gURLBar.value;
+  // trim leading/trailing spaces (bug 233205)
+  var url = gURLBar.value.replace(/^\s+/, "").replace(/\s+$/, "");

We were trimming too late, so the later regex checking for starting ^http would fail to fail if there were leading spaces. It shouldn't be an issue if getShortcutOrURI receives a trimmed url as input.


-  if (!/^(www|http)|\/\s*$/i.test(url) &&
+  if (!/^https?:\/\//i.test(url) &&

Don't explicitly check for www. here.
Add check for https.
Require :// to follow http or https in case someone wanted to go to http:8080 for some reason.
Remove check for trailing slash which is undesired and doesn't work correctly now that we handle slashes later on. Trailing slash was added in bug 177618 so that a url like mozilla.org/ would not trigger fixing. Removing it allows "mozilla/" ctrl-enter to work as expected.


+    var prefix = "www.";

For now hardcode the desired prefix to be "www.". [But with so many other changes, I wonder why I didn't just shove a try/catch gPref here...]

-      url = "http://www." + url;
+      // Prepend the prefix. nsDefaultURIFixup::CreateFixupURI will add protocol.
+      if (url.indexOf(prefix) != 0)
+        url = prefix + url;

Add the prefix only if it doesn't already exist. If the prefix is "ftp.", I CreateFixupURI will use the ftp:// protocol. If the prefix is blank, the if won't pass, but adding wouldn't change it either. (So if the prefix is "ftp." "mozilla" ctrl-shift-enter will eventually end up at "ftp://ftp.mozilla.org/" instead of "http://ftp.mozilla.org/")
** Issue ** "mozilla" ctrl-enter shows "www.mozilla.com/" until it actually loads then shows "http://www.mozilla.com/" in the location bar. I doubt it's related to bug 310651, and my little hack fix for that bug doesn't solve the problem that "http://" doesn't show up.

Additional concerns..
1) The suffix and prefix currently does a case sensitive check against the url.
2) Is the trailing slash really needed in the suffix if nsURIFixup (might??) add it
3) Perhaps I should have found a better place for discussion?

Comment 4

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

Comment on attachment 222910 [details] [diff] [review]
allow words starting with www to be completed

It'd be great to get unit tests for this behavior, though I guess that might depend on bug 375469.

Comment 5

[Nickolay_Ponomarev]

The diff doesn't apply cleanly and I'm not up to making sense of this change (looks like just the regexp was tweaked) and testing it in the newer build. If you could attach an updated (tested) patch, I can check it in.

Comment 6

[Simon Bünzli]

Attachment: fix (unbitrotted)

Comment 7

[Nickolay_Ponomarev]

Thanks! Checked in:

mozilla/browser/base/content/browser.js  1.783