247238 - Command-Enter does not open new tab/window from URL bar or search bar

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, enhancement)

Description

[Jim]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040514
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040514

When manually entering a URL, holding Command while hitting enter only beeps,
releasing command enters URL into current tab. A new tab can only be opened
manually.

Reproducible: Always
Steps to Reproduce:
1. Enter URL manually into address bar
2. Hold down command key and press enter
3.
Actual Results:  
System beeps, and nothing happens

Expected Results:  
Open a new tab with the requested URL as in Mozilla or Safari

Comment 1

[Stuart Morgan]

*** This bug has been marked as a duplicate of 164863 ***

Comment 2

[Stuart Morgan]

Oops, didn't read carefully enough. Bug 164863 is about hitting command-enter on
a selected link. Reopening.

This should also work in the search bar, and it should open a new tab or window
according to user preference. This is a slick feature that I always miss after
using FF or Safari, so I'm nominating it for 0.9

Comment 3

[Ludovic Hirlimann [:Usul]]

still true with 2004090408 (v0.8+)

Comment 4

[Geoff "temporarily off bugmail" Beier]

Attachment: implements command-return for search bar, location bar

This patch implements the requested feature for command-return in both the
location bar and the search bar. It respects the user's prefs for
command-clicking. That is to say, if command-click opens a link in a new
window, command-return opens a search or url in a new window. If command-click
opens in a new tab, command-return does likewise. If new windows/tabs open in
the background, commmand-return generated new windows/tabs obey this.

This is my first cut, and there is some code duplication between the
command-return handler and the search/location handlers. The right way may be
to refactor those to minimize duplication, but this is far less invasive. Let
me know in the review if I should do the more invasive refactoring instead.

Comment 5

[Geoff "temporarily off bugmail" Beier]

Attachment: version 2

This version of the patch has exactly the same functionality but incorporates
Mike's comments from IRC and performs the refactoring instead of duplicating
code.

Comment 6

[Ludovic Hirlimann [:Usul]]

Comment on attachment 159829 [details] [diff] [review]
version 2

looks good to me

Comment 7

[Mike Pinkerton (not reading bugmail)]

+typedef enum {
+  kDestinationNewWindow = 0,
+  kDestinationNewTab,
+  kDestinationCurrentView
+} BWCOpenDest;

nit. you don't need to typedef in C++, just C. this can be written as

enum BWCOpenDest { ... } ;

+-(void)performSearch:(SearchTextField *)searchField
inView:(BWCOpenDest)destination inBackground:(BOOL)loadInBG

name your params with in/out/io prefix to show they're parameters and not local
variables.

+    // revert the text in the URL bar since it goes into a new tab/window
+    [mURLBar revertText];

is this sufficient to get back to the previous url? what if they made other
changes before clearing it and typing this url? i'd say you should get the url
from the page and set it to that.

Comment 8

[Geoff "temporarily off bugmail" Beier]

Attachment: version 3

this should address pinkerton's comments.

Comment 9

[Geoff "temporarily off bugmail" Beier]

Comment on attachment 160362 [details] [diff] [review]
version 3

moving sr request to the updated patch

Comment 10

[Mike Pinkerton (not reading bugmail)]

landed.

Comment 11

[Jasper]

*** Bug 164863 has been marked as a duplicate of this bug. ***

Comment 12

[Geoff "temporarily off bugmail" Beier]

marking sr+ since pink sr'd and landed, to get it off "my requests".