162791 - PATCH: URL bar accepts Drag & Drop of strings/URLs

Status: VERIFIED FIXED

(Camino Graveyard :: Location Bar & Autocomplete, defect)

Description

[David Haas (gone, not reading mail)]

Currently, when the URL bar is NOT being edited, it refuses all drag & drops.
Patch fixes that.

Comment 1

[David Haas (gone, not reading mail)]

Attachment: D&D in URL bar

If you're not editing the URL bar, and drag text or a URL into it, the URL bar
contents are entirely replaced by what you dropped in & Chimera immediately
tries to load it (a la IE & OmniWeb).  This is a "new feature" of this patch -
currently, the URL bar rejects all drops if it's not being edited.

This patch does not change the behavior listed in bug 161053.  That bug only
comes into play if you ARE editing the URL bar, and drag text or a URL into it.
 Then (currently) it does the standard text field thing of inserting the
dragged text/URL at the caret without erasing what's currently there.  See that
bug for further comments.

This also doesn't fix bug 161856, although it'd be basically the same code
there as here.

Comment 2

[Mike Pinkerton (not reading bugmail)]

+  if ( [[pboard types] containsObject:NSURLPboardType] )
+    dragString = [[NSURL URLFromPasteboard:pboard] absoluteString];

we saw a lot of problems with NSURL throwing cocoa exceptions when given a url
with certain characters. Would this do the same thing? We had to stop using
NSURL for our url dispatch api's because of this.

Comment 3

[David Haas (gone, not reading mail)]

Comment on attachment 95352 [details] [diff] [review]
D&D in URL bar

Yea, I remember that - but if I'm not mistaken, the problems came when
converting a string into an NSURL. It didn't automatically properly escape all
the characters it needed, so we'd just get nil back instead of a valid NSURL. 
Does that sound right, or was there something else, too?

Anyway, since this code is just for accepting drags, any NSURL that it stumbles
across should be a valid NSURL, or else I don't think it would have been added
to the pasteboard as a draggable item . . .

Which (while I'm sitting here at an OS 9 box so unable to check code or do a
quick test) makes me think of two things - does any of the other drag accepting
code take NSURL's?  If not, and the problem with NSURL's was just from the
creation, perhaps it should be added in as a valid drag type in other code?

Also - I noticed in the cocoa-dev mailing list today how to correctly accept
.weblocs as a useful drag type, so I'd want to add that in here before getting
this applied.

Comment 4

[David Haas (gone, not reading mail)]

Attachment: updated drag&drop

Patch updated to handle weblocs as well.

I stole a constant of of NSPasteboard+Utils.mm, but had to change it to avoid a
linking error.	If the constant were moved to Pastboard+Utils.h, I could just
include the header & get rid of the declaration.

Comment 5

[David Haas (gone, not reading mail)]

Attachment: even better

i made kCorePasteboardFlavorType_url into a static
in both NSPasteboard+Utils & AutoCompleteTextField so that
I could use the same variable name, and added in one more
check for that flavor.	otherwise it's the same patch as before.

Comment 6

[Mike Pinkerton (not reading bugmail)]

will this patch cause you to load the url on a drop? Is this the behavior we
want? if you drag something to the url bar rather than the content area, isn't
the expectation that you want to do some editing/tweaking?

other than that, looks ok. simon, can you peek at this just for sanity?

Comment 7

[David Haas (gone, not reading mail)]

Yes, it loads the URL on a drop.  I like this method the best.
Here's what other browsers do:

Safari loads the URL on drop.
Mozilla inserts the URL as text (without clearing the URL bar of other text - ugly).
IE on OS 9 leaves the URL in for editing.
IE on OS X doesn't do anything at all with the URL.

Comment 8

[Simon Fraser [no longer active]]

I'm not sure we want to load the url; that's too destructive.

Comment 9

[David Haas (gone, not reading mail)]

Attachment: doesn't load URL on drop

Doesn't load the URL on drop - just selects it.

I prefer the load on drop behavior (ie, the "even better" patch).  But
whatever.

Comment 10

[Mike Pinkerton (not reading bugmail)]

bumping up, patch

Comment 11

[Ludovic Hirlimann [:Usul]]

Comment on attachment 122076 [details] [diff] [review]
doesn't load URL on drop

>+    if (sourceDragMask & NSDragOperationCopy) {
>+      return NSDragOperationCopy;

Shouldn't this be if (sourceDragMask && NSDragOperationCopy) ?

Else the patch applies cleanly and works has advertized.

Comment 12

[David Haas (gone, not reading mail)]

comment 11: It's checking to see if the NSDragOperationCopy bit is set in
sourceDragMask.  That's why it's & instead of && 

Comment 13

[Ludovic Hirlimann [:Usul]]

Comment on attachment 122076 [details] [diff] [review]
doesn't load URL on drop

Thanks david for the explanation.

Comment 14

[S Woodside]

what is this?

+static const NSString* kCorePasteboardFlavorType_url  =
@"CorePasteboardFlavorType 0x75726C20"; // 'url '  url

Comment 15

[S Woodside]

re comment #8 ... it's not destructive because you can always "go back" . I
think it should load the URL as well.

Comment 16

[Simon Fraser [no longer active]]

+static const NSString* kCorePasteboardFlavorType_url  =
@"CorePasteboardFlavorType 0x75726C20"; // 'url '  url

This is how cocoa identifies carbon drag flavors (which use four-char codes,
rather than strings).

Comment 17

[S Woodside]

Comment on attachment 122076 [details] [diff] [review]
doesn't load URL on drop

r+

Comment 18

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 122076 [details] [diff] [review]
doesn't load URL on drop

sr=pink

Comment 19

[Mike Pinkerton (not reading bugmail)]

fixed

Comment 20

[Chris Petersen]

With the 2003112613 build, dragging a URL to the url bar does replaces the
previous content with the new URL. The newly dropped URL becomes selected but
isn't loaded automatically. I personally like this behavior of it automatically
loading after the drag n drop.