299799 - Add "copy URL to clipboard" contextual menu item to History

Status: VERIFIED FIXED

(Camino Graveyard :: History, enhancement)

Description

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

Unlike in Bookmarks, there's no easy way to copy a visited URL elsewhere.  The
URL is not editable like in Bookmarks and there's no Get Info inspector, either.
 The only ways to copy a URL (for pasting into a textbox or other program) are
either 1) visist the site or 2) drag the history item to the URL bar and then copy.

A "Get Info" inspector could display info about the history item (as in the
columns, as done by the Bookmarks inspector) and let users copy the info by not
edit it.

A "copy URL to clipboard" contextual menu item/"action" button item would do
just that on any selected History item.

Come to think of it, "copy URL to clipboard" would be useful in the Bookmarks
contextual menu/"action" button, too.  Nothing in existing bugs about that.

Comment 1

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

We should add this to the "action" button's menu, too, of course, to stay in
sync with bookmarks.

Copy URL to clipboard might be contextual menu overload (esp. if we add Get Info
to History)?

Comment 2

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

(In reply to comment #0)
> Come to think of it, "copy URL to clipboard" would be useful in the Bookmarks
> contextual menu/"action" button, too.  Nothing in existing bugs about that.

Er, that would be bug 254313 :oops:

Comment 3

[Bruce Davidson]

I'll take this bug along with the one on bookmarks.

Is a full "get info" inspector actually useful for anything other than allowing you to copy the URL? If not its probably not worth adding, the context menu option is a much better way of copying the URL.

Comment 4

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

If we ever allow users to hide columns in the manager, a Get Info inspector could show all the info, but since I don't think there's a bug for that, we can cross that bridge if we get there.  

The only other thing I can see it having is if we currently track the number of visits each history item has; the columns only display last and first visit.

Still, if it's warranted, someone can file a new bug for the inspector.  The CM will solve my problem and keeps to the spirit of simplicity :-)

Comment 5

[Chris Lawson (gone)]

Bruce, any progress on this? I just realised how useful this would be today.

If you're not actively working on a patch, I might be able to whip something up in the next couple weeks.

cl

Comment 6

[Chris Lawson (gone)]

Having heard nothing from Bruce since Monday, taking this and bug 254313.

Comment 7

[Chris Lawson (gone)]

Question.

In History, and *only* in History, we have disabled Copy in the Edit menu.

Why?

It works in the Bookmarks pane of the Bookmarks Manager.

I ask because cmd-C is a useful shortcut for copying the bookmark item URL (see bug 254313) and works great there, and I'd like it to be available in History, too. I want to know why we disabled Copy in History, though, before I go re-enabling it.

cl

Comment 8

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

(In reply to comment #7)
> I want to know why we disabled Copy in History, though, before I go
> re-enabling it.
I assume it's disabled because it's never been hooked up; see bug 302866, bug 155484 (esp. bug 155484 comment 5), and bug 280028.

Comment 9

[Simon Fraser [no longer active]]

When you Copy a bookmark item, you can actually paste that item somewhere else. That's not true for History, but copying the URL could make sense. But what if I select > 1 and Copy?

Comment 10

[Chris Lawson (gone)]

(In reply to comment #9)
> When you Copy a bookmark item, you can actually paste that item somewhere else.
> That's not true for History

You could copy a History item into Bookmarks, I suppose.

>, but copying the URL could make sense. But what if
> I select > 1 and Copy? 

Same thing that happens if you select more than one bookmark? Ideally, anyway.

cl

Comment 11

[Chris Lawson (gone)]

Attachment: pizzatch, version1

Comment 12

[Chris Lawson (gone)]

Attachment: version 2, now with less debug code

Oops. Left an NSLog() in there. That's been removed now. :)

Comment 13

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 205280 [details] [diff] [review]
version 2, now with less debug code

sr=pink. looks ok...

one nit

+  for ( unsigned int i = 0; i < [historyItemsToCopy count]; ++i ) {

avoid the extra spacing around the parens should just be:

for (unsigned int....++i)

Comment 14

[Simon Fraser [no longer active]]

Comment on attachment 205280 [details] [diff] [review]
version 2, now with less debug code

>Index: HistoryOutlineViewDelegate.h
>===================================================================
>RCS file: /cvsroot/mozilla/camino/src/history/HistoryOutlineViewDelegate.h,v
>retrieving revision 1.5
>diff -u -8 -r1.5 HistoryOutlineViewDelegate.h
>--- HistoryOutlineViewDelegate.h	29 Jun 2005 01:22:14 -0000	1.5
>+++ HistoryOutlineViewDelegate.h	8 Dec 2005 05:52:59 -0000
>@@ -69,16 +69,19 @@
> 
> - (IBAction)openHistoryItem:(id)sender;
> - (IBAction)openHistoryItemInNewWindow:(id)aSender;
> - (IBAction)openHistoryItemInNewTab:(id)aSender;
> - (IBAction)openHistoryItemsInTabsInNewWindow:(id)aSender;
> 
> - (IBAction)deleteHistoryItems:(id)sender;
> 
>+- (IBAction)copyURLs:(id)sender;
>+- (void) copyHistoryURLs:(NSArray*)historyItemsToCopy toPasteboard:(NSPasteboard*)aPasteboard;

Match the spacing of surrounding methods

>Index: HistoryOutlineViewDelegate.mm
>===================================================================

>+//
>+// Copy a set of history URLs to the specified pasteboard.
>+// We don't care about item titles here, nor do we care about format.
>+//
>+- (void) copyHistoryURLs:(NSArray*)historyItemsToCopy toPasteboard:(NSPasteboard*)aPasteboard
>+{
>+  // handle URLs, and nothing else, for simplicity.
>+  [aPasteboard declareTypes:[NSArray arrayWithObject:kCorePasteboardFlavorType_url] owner:self];
>+
>+  NSMutableArray* urlList = [NSMutableArray array];
>+  for ( unsigned int i = 0; i < [historyItemsToCopy count]; ++i ) {

An NSEnumerator would be cleaner.

r=me, but feel free to udpate the patch before someone checks it in.

Comment 15

[Chris Lawson (gone)]

Attachment: fixed per Simon's comments

Fixed per comments, moved review flags.

Comment 16

[Chris Lawson (gone)]

Attachment: fixes a missing cast to avoid a compiler warning

...and one more update before checkin.

cl

Comment 17

[Simon Fraser [no longer active]]

In on trunk and branch.