364198 - Allow the favicon (site icon) to act as a proxy for local files

Status: RESOLVED FIXED

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

Description

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

In lieu of bug 363633, Ian's worked up a patch that allows the favicon to act as a proxy for local files, so you get all the proxy-for-actual-file benefit with none of the icon duplication.

Comment 1

[froodian (Ian Leue)]

Attachment: Patch

- In NSPasteboard+Utils declared NSFilenamesPboardType as a pboard type and puts file urls on the pboard as NSFilenamesPboardType
- Enables link and copy drags (if not already present)
  -from the location bar, bookmarks bar, tab favicons, bookmarks manager, and history
  -to external apps.

We should look at the external-app logic changes carefully to make sure it doesn't allow anything we don't want through.

Comment 2

[Stuart Morgan]

Comment on attachment 248972 [details] [diff] [review]
Patch

I haven't had time to play with behavior yet, but a couple of questions:

>+  NSMutableArray* filePaths = [NSMutableArray array];
>+  for (unsigned int i = 0; i < urlCount; ++i) {
>+    NSURL* url = [NSURL URLWithString:[inUrls objectAtIndex:i]];
>+    if ([url isFileURL])
>+      [filePaths addObject:[url path]];
>+  }

If we are dragging 10 things, and only one is a file, will the other 9 be ignored entirely as a result of this?

>+  [self setPropertyList:filePaths forType:NSFilenamesPboardType];

Along the same lines, could advertising this type with an empty array as contents (in the case where nothing is a file) cause apps to do nothing, rather than handle one of the other flavors?

> - (unsigned int)draggingSourceOperationMaskForLocal:(BOOL)flag
> {
>-	return NSDragOperationGeneric | NSDragOperationCopy;
>+	return (NSDragOperationGeneric | NSDragOperationCopy | NSDragOperationLink);
> }

Shouldn't NSDragOperationLink only be when dragging non-locally?

Comment 3

[Stuart Morgan]

Comment on attachment 248972 [details] [diff] [review]
Patch

I can't find any cases where non-local URL arrays work for more than one thing anyway, so I guess dragging a mix wouldn't be any more broken than dragging a bunch of URLs. Looks good otherwise, so r=me (I'll leave it up to you whether all the NSDragOperationLink returns need to be non-local only).

>-	return NSDragOperationGeneric | NSDragOperationCopy;
>+	return (NSDragOperationGeneric | NSDragOperationCopy | NSDragOperationLink);

You may as well fix the tab while you are touching this line.

Comment 4

[froodian (Ian Leue)]

Attachment: Patch

(In reply to comment #3)
> (From update of attachment 248972 [details] [diff] [review])
> I can't find any cases where non-local URL arrays work for more than one thing
> anyway, so I guess dragging a mix wouldn't be any more broken than dragging a
> bunch of URLs. 

Exactly.  Sorry I didn't reply in the bug btw, I was trying to find a time to talk to you about that in channel.

> Looks good otherwise, so r=me (I'll leave it up to you whether
> all the NSDragOperationLink returns need to be non-local only).

That's the most logical. Done.

Comment 5

[Mike Pinkerton (not reading bugmail)]

 - (unsigned int)draggingSourceOperationMaskForLocal:(BOOL)flag
 {
-	return NSDragOperationGeneric | NSDragOperationCopy;
+  if (flag)
+    return (NSDragOperationGeneric | NSDragOperationCopy);
+
+  return (NSDragOperationGeneric | NSDragOperationCopy | NSDragOperationLink);

can you add a function-level comment and specify what |flag| does, especially now that we're using it for something? and rename it to |isLocal| like i see elsewhere in the diff?

sr with that.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 252021 [details] [diff] [review]
Patch

forgot to mark, see my comment about adding more comments and then sr=pink

Comment 7

[froodian (Ian Leue)]

Checked in on trunk and MOZILLA_1_8_BRANCH