Closed Bug 617361 Opened 14 years ago Closed 1 month ago

Text consisting of lists of URLs cannot be dragged into Bookmark Manager

Categories

(Camino Graveyard :: Bookmarks, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: bugzilla-graveyard, Unassigned)

References

()

Details

(Whiteboard: p-safari)

STR:

1) Select some text consisting of a return-separated list of URLs, like so:

http://foo.bar
https://bar.baz
http://baz.foo

2) Drag into the Bookmark Manager.

ER: Create one bookmark, in whatever collection or folder the drag ends in, for each line in the dragged text.

AR: If drag ends on a collection, Camino creates a single bookmark with a URL of "http://foo.barhttps://bar.bazhttp://baz.foo", stripping all the whitespace from the dragged text and turning it into one single (broken) URL. If the drag ends on a folder in the main table view, nothing happens at all. Neither of these behaviours seem to be useful, and we should at least be consistent if we're going to be broken :-p

Safari properly creates one bookmark for each line in the dragged text no matter where the drag terminates. (As a workaround, the resulting Safari bookmarks can then be dragged into Camino, but we ought to handle this in the first place.)
Note that in 1.6.x, drags over the collections are also rejected; looks like that regressed (from the point of view of consistency between collections, Manager, and Bookmarks Bar) in 2.x.

On irc, I said I though that the paste-sanitizing formatter (which I think is what is involved here, given the result for a collection) should check to see if the pasteboard object has multiple valid URLs and, if so, add a new flavor to the object that has a dict of bookmark items or WebURLsWithTitlesPboard or whatever.  (I haven't looked at our pasteboard code in a while, so my conception is fuzzy and may be wrong, but I think that's possible.)  Then the "bookmarks system" should act on the drag just like it acts on an inter-bookmarks drag of multiple items or a drag of multiple items from Safari's bookmarks manager (or Chrome's).
(In reply to comment #0)
> If the drag ends on a folder in the main table view, nothing happens at all.

So, I can't reproduce that at all.

In 1.6.11, drags anywhere (bar, outline view, outline view folder, collections) are consistently rejected.

In 2.1a1pre, drags anywhere consistently create the awesome bookmark "http://foo.barhttps://bar.bazhttp://baz.foo"
To summarize from irc:

NSPasteboard+Utils's |containsURLData| probably needs an additional, smarter check when it handles NSString data:

* It needs to differentiate between
** "starts with a valid-looking URL and whitespace, but does not appear to contain any other URL data after the first occurrence of whitespace"
** "starts with a valid-looking URL and whitespace, and possibly contains additional valid-looking URLs"
** "starts with a potentially valid-looking URL (if whitespace is cleaned up)"

Then presumably |getURLs:andTitles| would do the right thing (perhaps with a little help required where it handles NSString data; unclear).

We don't want to break our fixup of things like "http://fo o.com/" and "http://b
ar.com/" and similar in order to correctly handle lists of URLs, but we also don't want to make bookmarks like 
"http://foo.barhttps://bar.bazhttp://baz.foo", either.

Maybe: if the string starts with potentially valid-looking URL, process and strip whitespace, but if start of the next "hunk" after whitespace looks like a protocol/potentially valid-looking URL fragment, treat that whitespace as a dividing point to be preserved?  We'd still end up with false positives from something like "http://caminobrowser.org/ is awesome" though :( (creating a URL of "http://caminobrowser.org/isawesome" as a bookmark, though that matches what we do on paste in the location bar, so that might be OK). 

Incidentally, how does "http://foo.barhttps://bar.bazhttp://baz.foo" pass an isValidURI check?  Smells like another bug.

At the very least, creating a bookmark of "http://foo.barhttps://bar.bazhttp://baz.foo" from a list of URLs is a bug and we should fix something, so confirming.  Whether we fix things to parse into a set of URLs (I think we should) or just make that fail isValidURI, or both, can be determined.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to comment #3)
> Incidentally, how does "http://foo.barhttps://bar.bazhttp://baz.foo" pass an
> isValidURI check?  Smells like another bug.

I retract that; we've supported making bookmarks out of any string for a while now (e.g., drag "Camino" to the bmbar in 1.6.x and we make a bookmark "Camino" with URL "Camino"); I remember us going back and forth between too tight of validation and too loose of validation as far back as pre-1.0, trying to find the right mix (e.g., bug 313027).
(For reference, the fancy location-bar sanitizing code is apparently BWC's
|cleanPastedText:|
http://mxr.mozilla.org/camino/source/camino/src/browser/BrowserWindowController.mm#296  I'm not sure if it's involved here or not.)
(In reply to comment #5)
> (For reference, the fancy location-bar sanitizing code is apparently BWC's
> |cleanPastedText:|
> http://mxr.mozilla.org/camino/source/camino/src/browser/BrowserWindowController.mm#296
>  I'm not sure if it's involved here or not.)

It's not.
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.