635064 - Can't drag a plain-text bookmark shortcut URL to create a bookmark

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

STR:

1) Have text http://www.google.com/search?q=%s (or paste it in the location bar)
2) Select said text URL and drag to the bookmark bar or somewhere in the manager

ER: Bookmark created
AR: Nothing happens

This might be on purpose (failing some sort of NSURL validation somewhere?); not sure.  I'll try to investigate when I have more time to flail around in our drag/pasteboard/bookmarks code.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Once it's been linkified, as it is now in comment 0, dragging works fine, so I'm more inclined to consider this a bug due to the inconsistency (and also since we allow random plain-text strings to be dragged).

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Yeah, what happens here is that we fail the isValidURI check when we call it here: http://mxr.mozilla.org/camino/source/camino/src/extensions/NSPasteboard+Utils.mm#285

pasteboard string: http://www.google.com/search?q=%s
potentialURLString: http://www.google.com/search?q=%s
URL: (null)

The linkified URL drag works because that has gotten the '%' properly encoded (and it's already URL data, not NSString).

I don't know if we should pervert that validation method some more (isBookmarkShortcutURL?) or if that's even possible to know definitively if we have a shortcut (that's one of the issues hendy was having with UTF8 bookmarks URL display, iirc): http://mxr.mozilla.org/camino/source/camino/src/extensions/NSString+Utils.m#97

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #2)
> The linkified URL drag works because that has gotten the '%' properly encoded
> (and it's already URL data, not NSString).

But those don't actually produce workable shortcut URLs, since the '%' is already encoded and we don't decode it!

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Make plain-text shortcut URLs pass URL validation

Adds |isBookmarkShortcutURI| and has |isValidURI| check that when doing its validation.

Comment 5

[Stuart Morgan]

Comment on attachment 514311 [details] [diff] [review]
Make plain-text shortcut URLs pass URL validation

sr=smorgan

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/d655f796ced0