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

RESOLVED FIXED in Firefox 3 alpha5



13 years ago
12 years ago


(Reporter: nima.goudarzi, Assigned: zeniko)


Firefox 3 alpha5

Firefox Tracking Flags

(Not tracked)



(1 attachment, 2 obsolete attachments)



13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060426 Firefox/
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060426 Firefox/

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)

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

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

Comment 1

13 years ago
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.
Ever confirmed: true


13 years ago
Severity: major → minor
OS: Windows XP → All
Hardware: PC → All

Comment 2

13 years ago
Created attachment 222910 [details] [diff] [review]
allow words starting with www to be completed

... and also adding https to the list of non-completable words.
Assignee: nobody → zeniko
Attachment #222910 - Flags: review?(mconnor)

Comment 3

12 years ago
Created attachment 242477 [details] [diff] [review]
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?
Attachment #242477 - Flags: review-


12 years ago
Attachment #222910 - Flags: review?(mconnor) → review?(gavin.sharp)
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.
Attachment #222910 - Flags: review?(gavin.sharp) → review+
Whiteboard: [checkin needed]
Target Milestone: --- → Firefox 3 alpha5
Attachment #242477 - Attachment is obsolete: true

Comment 5

12 years ago
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.
Whiteboard: [checkin needed] → [checkin needed] (patch needs manual merging)
Version: unspecified → Trunk

Comment 6

12 years ago
Created attachment 264672 [details] [diff] [review]
fix (unbitrotted)
Attachment #222910 - Attachment is obsolete: true


12 years ago
Whiteboard: [checkin needed] (patch needs manual merging) → [checkin needed]

Comment 7

12 years ago
Thanks! Checked in:

mozilla/browser/base/content/browser.js  1.783
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
You need to log in before you can comment on or make changes to this bug.