Last Comment Bug 303628 - Bookmark keywords with spaces gives "Invalid URL" error if entered in address bar (do input validation on keyword entry)
: Bookmark keywords with spaces gives "Invalid URL" error if entered in address...
Status: RESOLVED FIXED
[good first bug]
: fixed1.8.1.1
Product: Camino Graveyard
Classification: Graveyard
Component: Bookmarks (show other bugs)
: unspecified
: PowerPC Mac OS X
-- normal (vote)
: Camino1.5
Assigned To: Stuart Morgan
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-08-05 16:38 PDT by froodian (Ian Leue)
Modified: 2006-11-06 08:01 PST (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
prevent spaces (5.41 KB, patch)
2006-10-26 07:53 PDT, Stuart Morgan
bugzilla-graveyard: review+
Details | Diff | Splinter Review
with comment (5.43 KB, patch)
2006-11-06 07:53 PST, Stuart Morgan
stuart.morgan+bugzilla: superreview+
Details | Diff | Splinter Review

Description User image froodian (Ian Leue) 2005-08-05 16:38:43 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050531 Camino/0.9+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050531 Camino/0.9+

If you make a bookmark with a keyword with a space (eg "this keyword"), then try
to call that bookmark by entering the keyword in the address bar, Camino tries
to read the keyword as a URL.

Reproducible: Always

Steps to Reproduce:
1. Make a new bookmark to www.google.com, giving it a keword of "this keyword"
2. Type "this keyword" in address bar
Actual Results:  
Sheet error, saying that the URL is not valid.

Expected Results:  
either bring up the bookmark, or don't allow the keyword to have a space in the
first place.
Comment 1 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-08-06 04:00:44 PDT
Are keywords even allowed to contain spaces?  I couldn't find any documentation
that addressed the issue.

Otherwise, it sounds a little like bug 197918.
Comment 2 User image Ludovic Hirlimann [:Usul] 2005-08-08 10:37:28 PDT
(In reply to comment #1)
> Are keywords even allowed to contain spaces?  I couldn't find any documentation
> that addressed the issue.
> 
> Otherwise, it sounds a little like bug 197918.
-(NSArray *)resolveBookmarksKeyword:(NSString *)keyword only knows how to deal
with two words maximum.

-(NSArray*)resolveKeyword:(NSString *)keyword withArgs:(NSString *)args

seems to be our problem.

I say we add a note to the release note about that issue, or we enforce the one
word thingy when creating such an entry in the BM.
Comment 3 User image Samuel Sidler (old account; do not CC) 2005-08-08 10:50:20 PDT
Simon and I talked about this on IRC.

We don't allow two-word keywords because the second word is always assumed to 
be a string to replace %s (or whatever it is). This is a core issue, but it's 
done on purpose. We could, however, change the error message.

Otherwise, this is a WONTFIX.
Comment 4 User image froodian (Ian Leue) 2005-10-14 16:22:41 PDT
What's the status on this bug?  Is it WONTFIX?  I'd prefer if the error message
were changed, since as it is it's unclear to the user what's going on.  

Even better than that (in my mind) would be an error message that comes up when
the user first tries to make a keyword with a space in it.
Comment 5 User image Samuel Sidler (old account; do not CC) 2005-10-27 13:04:52 PDT
If nothing else, we should change the dialog that comes up when a user attempts to go to a two-word keyword bookmark.

CCing Simon, targeting for 1.1.
Comment 6 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-05-11 03:33:56 PDT
We should do input validation on the keyword field in the manager proper and in the Info panel.  Validating for lack of whitespace should be very easy, just a few lines of code per irc.

If we can fix the error sheet ("The URL is not valid and cannot be loaded."), too, we should. My guess it comes from Necko, though, or that we get a generic error message that can't be mapped to this problem specifically.

Yep, it's a chrome string for malformed URL: http://lxr.mozilla.org/mozilla/source/dom/locales/en-US/chrome/appstrings.properties#37 (which sounds like it's fairly generic, too!)
Comment 7 User image Stuart Morgan 2006-10-26 07:53:51 PDT
Created attachment 243635 [details] [diff] [review]
prevent spaces

Just say no to spaces.
Comment 8 User image Chris Lawson (gone) 2006-10-27 22:06:15 PDT
Comment on attachment 243635 [details] [diff] [review]
prevent spaces

r=cl
Comment 9 User image Mike Pinkerton (not reading bugmail) 2006-11-06 06:49:40 PST
the class declaration needs comments to say what it's for and what it does.

sr=pink with that updated.
Comment 10 User image Stuart Morgan 2006-11-06 07:53:46 PST
Created attachment 244805 [details] [diff] [review]
with comment

With comment, as checked into trunk and MOZILLA_1_8_BRANCH

Note You need to log in before you can comment on or make changes to this bug.