keyboard shortcut to open selected bookmark

VERIFIED FIXED

Status

Camino Graveyard
Bookmarks
VERIFIED FIXED
15 years ago
14 years ago

People

(Reporter: S Woodside, Assigned: Mike Pinkerton (not reading bugmail))

Tracking

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

15 years ago
In bookmarks panel, return key opens the selected bookmark. Keyboard
accessibility, mainly.
(Reporter)

Comment 1

15 years ago
Created attachment 125987 [details] [diff] [review]
patch to BookmarksOutline view and Extended outlineView

There was already a hook in ExtendedOutlineView for mDoubleAction, so I
implemented that. First it checks that there's only 1 item selected. This
applies to history as well as long as it appears in the BookmarksOutlineView as
well.
(Reporter)

Updated

15 years ago
Attachment #125987 - Flags: review?(pinkerton)

Comment 2

15 years ago
->pink
Assignee: sfraser → pinkerton
(Assignee)

Comment 3

15 years ago
+-(void)setDoubleAction: (SEL)aDoubleAction;
+{
+  mDoubleAction = aDoubleAction;
+}
+
+-(SEL)doubleAction;
+{
+  return mDoubleAction;
+}
+

isn't there already a doubleAction/setDoubleAction on NSTableView (grandparent
of our outline view) that does all this for us? I think all you need is the
keyDown method. Can you try it and verify?
(Reporter)

Comment 4

15 years ago
Created attachment 126921 [details] [diff] [review]
patch BookmarksOutlineView.mm

You're right. I deleted that part and it still works.
(Reporter)

Updated

15 years ago
Attachment #125987 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #126921 - Flags: review?(pinkerton)
(Reporter)

Updated

15 years ago
Attachment #125987 - Flags: review?(pinkerton)
(Assignee)

Comment 5

15 years ago
after looking at safari, we mimic their behavior which keys an inline edit. it
may even be built into the NSTableView that way. thoughts?
(Reporter)

Comment 6

15 years ago
safari is wrong ? ;-)
(Assignee)

Comment 7

15 years ago
i'm really torn. return to open seems to be the most logical choice here, but
the default behavior in cocoa seems to be to edit. The finder also behaves this
way. The HIG's are silent.
(Reporter)

Comment 8

15 years ago
There should be a way to open them with the keyboard whether it's return or
other keyboard shortcut.

Comment 9

15 years ago
Comment on attachment 126921 [details] [diff] [review]
patch BookmarksOutlineView.mm

We have a new Camino request flag which can be set multiple times for a patch.
Moving old review requests to the new flag. Sorry for the spam.
Attachment #126921 - Flags: review?(pinkerton) → review?

Comment 10

15 years ago
In Mac OS finder (X and classic):

return key to edit name
command-down arrow to open selected items

It'll be great if Camino use the same shortcuts. For the sake of consistency (and it's a drag to 
remember all shortcuts in all different apps.) ;)
Simon, pink is this patch still supose to be valide since dave's new bm landed ?
(Reporter)

Updated

15 years ago
Summary: return key opens selected bookmark → keyboard shortcut to open selected bookmark
(Reporter)

Comment 12

15 years ago
Created attachment 135594 [details] [diff] [review]
enter key

Uses the enter key instead of the return key.
Attachment #126921 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #126921 - Flags: review?
(Reporter)

Updated

15 years ago
Attachment #135594 - Flags: review?
Comment on attachment 135594 [details] [diff] [review]
enter key

>+      // open the bookmark
>+      if( [self numberOfSelectedRows] == 1 && [self doubleAction] )

Giving r+.
Waht about the multi selection case, the present patch does not work with that,
but double clicking on multiple selected bookmark does not work either.
Attachment #135594 - Flags: review+
Blocks: 223181
(Reporter)

Comment 14

15 years ago
I think that's a a good idea, I filed Bug 225915.
A couple things:

1st: The other special key processing off a key down event is handled in
ExtenededOutlineView.mm.  I'd suggest moving this logic into that file, rather
than in BookmarkOutlineView.mm.  

2nd: as per comment 10: Go with the Finder's behavior on this - "return" edits
the text, "command + down arrow" opens the bookmark.
*** Bug 164195 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 17

15 years ago
comment 15 . 1
I think that it should be in BookmarkOutlineView ... why do we even have both
ExtendedOutlineView and BookmarkOutlineView anyway? Since ExtendedOutlineView is
only used by bookmarks manager ... and contains bookmarks-specific code (but
lives  in src/extensions for some reason)

comment 15 . 2
OK, I'll implement Cmd-DownArrow as well. I'll keep EnterKey because I want a
one key press option.
(Reporter)

Comment 18

15 years ago
Created attachment 136398 [details] [diff] [review]
enter key, Cmd-DownArrow, and multiple bookmarks

I added the ability to open multiple bookmarks and included it here, because
there's no way to open multiple bookmarks without the keyboard shortcut anyway.
I could split them if that's important.
Attachment #135594 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #136398 - Flags: review?
Separate out the "multiple bookmark" part of the patch.  It's going to be more
complicated - should things open in a new window?  in new tabs?  Those prefs
need to be checked.  Etc.  And when you do that patch, you don't need to add in
a new method to "BrowserWindowController" - just used the already existing
openTabGroup, or if it really bugs you, globally change all mentions of
openTabGroup to openURLArray.

What's left is the changes to ExtendedOutlineView.  There - itemsToOpen is
leaking. release or autorelease it.  Otherwise works as promised.

As to why ExtenedOutlineView exists - I believe the historical reason was the
split between bookmarks & history in the sidebar.  The history code still
touches it a little.  At one point I think I remember looking at it and deciding
it still made some sense, but that might not be true (or true anymore).
sorry - one more thing 

+      if( [self numberOfSelectedRows] == 1 && [self doubleAction] ) {

I don't think you need the check for [self doubleAction].  
(Reporter)

Comment 21

15 years ago
Created attachment 136483 [details] [diff] [review]
enter & cmd-downarrow

Dumped the multi-bookmarks code. That'll be done in bug 225915. This is much
simpler now, I eliminated the check for just 1 element selected as well, since
it will only open one bookmark anyway ... it will be whichever is the target,
which is the last one you selected.
(Reporter)

Updated

15 years ago
Attachment #136398 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #136483 - Flags: review?
Attachment #136483 - Flags: review+
Attachment #135594 - Flags: review?
Attachment #136398 - Flags: review?
Attachment #136483 - Flags: review? → superreview?
Attachment #136483 - Flags: superreview? → superreview?(pinkerton)
(Assignee)

Comment 22

15 years ago
Comment on attachment 136483 [details] [diff] [review]
enter & cmd-downarrow

r=pink

i noticed that up/down arrow by themselves don't work. didn't they before?
maybe something to fix in the "open multiple" bug?
Attachment #136483 - Flags: superreview?(pinkerton) → superreview+
(Assignee)

Comment 23

15 years ago
landed
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
tested with 2005011808-trunk (10.3.7); looks great!

Return: puts selected bm in inline edit mode
Enter: opens selected bm in current page (or tab, if window has multiple tabs)
Cmd+Down arrow: opens selected bm in new tab (or window, depending on user's
pref setting)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.