286602 - Allow Copy to work on history items

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: Toolbars & Menus, enhancement, P3)

Description

[Simon Fraser [no longer active]]

The user should be able to copy info out of the history view.

Comment 1

[Bruce Davidson]

Taking this bug. Most of the underlying support was added in bug 155484, just
need to rig up the appropriate methods on the history view and its controller.

Comment 2

[Chris Lawson (gone)]

Bruce, any progress on this?

If you're not working on this any more, I'd like to kick it back to default and maybe have some of our newer guys tackle it.

cl

Comment 3

[Bruce Davidson]

Won't get to look at this in the near future so unassigning self.

Comment 4

[froodian (Ian Leue)]

-> me

Comment 5

[froodian (Ian Leue)]

Attachment: Patch

This turned out to be far more simple than I'd expected.  Just renamed the context menu selector to the standard name.

Comment 6

[Chris Lawson (gone)]

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

r-, and here's why:

|copyURLs:|, as its name implies, performs a specific action -- it copies the URLs of the selected history items.

History items are essentially a special-case BookmarkItem in Camino code, and if we're going to implement a full-scale copy, we need to implement it so that all their attributes, not just URLs, are put on the pasteboard, probably in the same way BookmarkViewController implements both |copy:| and |copyURLs:|.

See also here: http://lxr.mozilla.org/mozilla/source/camino/src/bookmarks/BookmarkViewController.mm#1123

cl

Comment 7

[Chris Lawson (gone)]

Oh, and the reason for doing this instead of making |copy:| simply use their URLs:

1) It's the Right Thing(tm) to do.
2) This is potentially a way to create new bookmarks. Pasting a URL into the Bookmark Manager right now *does* create a new bookmark, but without any title. Pasting a proper bookmark (or, after this is fixed, history item) in there preserves title and URL (and, presumably, other attributes, which I haven't looked into yet).

cl

Comment 8

[froodian (Ian Leue)]

Attachment: Somewhat Better

This is still less than ideal.  It copies and pastes the urls and titles of history items everywhere it should (including the bookmarks manager).

The URLs and titles are all it does though.  I'm not sure how bad this really is - AFAIK, the only missing piece of info that could be used by a "true" copy/paste is "Last visited."

The other issue is copying folders.  Right now, I have it beep if you try to copy a folder (but still copy any individual items selected), but that's a crappy solution.  Then again, I'm not sure why you'd want to copy a whole folder worth of history...

I'm perfectly willing to try to make this a bit better in these regards, but I'll need some pointers if that's to happen.

Comment 9

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

We should copy folders because 1) we copy them in bookmarks and 2) someone pointed me to this great site called weblogs.mozillazine.org and I visited a bunch of weblogs there and now I want an easy way to turn that folder of history items into bookmarks to visit daily ;)

(Copying the entire folder of sites visited today does seem silly, but not everyone groups history that way.)

cl got folders working in bookmarks; he might be able to help here.

Comment 10

[froodian (Ian Leue)]

Comment on attachment 224784 [details] [diff] [review]
Somewhat Better

Clearing reviews.  I have some ideas for this, but it should wait for the new bug that blocks this

Comment 11

[Chris Lawson (gone)]

Pretty sure Ian isn't working on this any more.

Wiring up the last-visited stuff ought to be fairly easy now that the blocker is fixed.

Comment 12

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

[sauron: cl: do you have any idea what ian might have been thinking of in bug 286602 comment 10 ?
[5:01pm] thebot: sauron: Bug https://bugzilla.mozilla.org/show_bug.cgi?id=286602 enh, P3, Future, nobody, NEW, Allow Copy to work on history items
[5:01pm] cl: let me open a window that doesn't have 572 tabs in it
[5:02pm] sauron: i was going through the old-patches query this weekend
[5:02pm] sauron: and found a couple of bugs where the perfect became the enemy of the good 
[5:02pm] cl: looking...
[5:03pm] sauron: (some would need some unbitrotting, but would otherwise be 90% of the way there, and it seems dumb to never have landed them  )
[5:05pm] cl: i have no idea what he might have been thinking of specifically, but now that we have a way to create bookmarks without a last-visited date of "now"
[5:05pm] cl: it should be fairly simple
[5:06pm] cl: since HistoryItems, IIRC, have a lastVisit attribute
[5:07pm] sauron: we'd have to copy them as a bookmark type, though, right, rather than as urls-and-titles?
[5:07pm] cl: so what we'd do is create a |newBookmarkItem = [Bookmark bookmarkWithTitle:itemTitle url:itemURL lastVisit:lastVisit]|
[5:07pm] cl: i didn't look to see how the copy was originally implemented
[5:08pm] sauron: a pair of nsmutablearrays
[5:09pm] cl: then yeah, why don't we just put them on the pasteboard as bookmarks?
[5:09pm] cl: we can't paste them into History anyway 
[5:09pm] sauron: well, i suppose in theory you could want to paste them into an email or text file
[5:10pm] cl: what happens if you do that right now with our bookmark type?
[5:10pm] cl: just URLs, right?
[5:10pm] sauron: i think
[5:10pm] cl: (IOW, it ignores last-visit dates, etc.)
[5:10pm] sauron: for RTF, we get a link
[5:10pm] sauron: for text fields, plain-text
[5:11pm] cl: so yeah, if we just use the history info to populate a bookmark item, and then put that on the clipboard, we'll do the right thing
[5:11pm] sauron: s/link/link with title as link text/
[5:12pm] • sauron wonders why drag works (modulo the date) and we had to implement copy separately 
[5:13pm] • cl does not remember enough Drag Manager/pasteboard code to answer that
[9:29pm] sauron: cl: we don't appear to ever put BookmarkItems on the pasteboard, only various interchange formats and the UUID of a BookmarkItem
[9:35pm] cl: oh. d'oh.
[9:35pm] cl: so how do bookmark drags work now?
[9:36pm] sauron: we apparently ask something for the data from the BookmarkItem whose UUID we have
[9:38pm] sauron: i tend to think we should just land froodian's patch as-is and unbitrotted, and spin off last-visit (which defaults to never in every existing case, i think) and folders
[9:39pm] ardissone left the chat room. (Quit: ardissone)
[9:39pm] ardissone left the chat room. (Quit: ardissone)
[9:39pm] sauron: i'll look some more once we've actually released 2.1

Comment 13

[Pallani Kumaran]

hello people, I would like to work on this as my first camino bug.