97123 - modifier+Enter in address field and go button should behave like modifier+click on link

Status: VERIFIED FIXED

(SeaMonkey :: Location Bar, enhancement, P1)

Description

[Matthew T (active 1999-2002)]

To reproduce:
1.  Enter an URL in Navigator's address field.
2.  Type Command+Enter (Mac) or Control+Enter (others).
3.  Focus the address field again, and type Shift+Command+Enter (Mac) or
    Shift+Control+Enter (others).
4.  Focus the address field again, and type Option+Enter (Mac) or Alt+Enter
    (others).
5.  Focus the address field again, and type Shift+Option+Enter (Mac) or
    Shift+Alt+Enter (others).

What should happen:
2.  The URL in the address field opens in a new window in front of the current
    window.
3.  The URL in the address field opens in a new window behind the current
    window (see bug 56690).
4.  A filepicker opens for specifying where to save the resource pointed to by
    the URL in the address field.
5.  The resource pointed to by the URL in the address field is saved to the
    most-recently-downloaded-to folder.

What actually happens:
2.  The URL is opened in the current window.
3.  The URL is opened in the current window.
4.  The URL is opened in the current window.
5.  The URL is opened in the current window.

Comment 1

[Matthew T (active 1999-2002)]

Oh, so we have an --> `URL Bar' component. Never noticed that before.

Comment 2

[Alec Flett]

reassign url bar bugs to new owner..

Comment 3

[Jesse Ruderman]

Would the URL bar for the first window be reset after I hit Ctrl+enter?  I 
think it should be, so that when I return to the first window, I don't get 
confused about what it's displaying.

Is there a bug for "Alt+Shift+clicking a link should download to most recent 
download location"?  When I Alt+Shift+click a link, half of the page becomes 
selected and I get a file picker.

Comment 4

[Jonas Jørgensen]

Shouldn't this also be for modifier+go button?

Comment 5

[Matthew T (active 1999-2002)]

> Would the URL bar for the first window be reset after I hit Ctrl+enter?

Yes it would.

> Shouldn't this also be for modifier+go button?

Yes it should. Blake, do you want a separate bug for that?

Comment 6

[Chris Lee]

I see an option to open in new tab is not yet included.
This looks like a whole lot of modifiers!
Would it not be usefull to have a go button that can be asigned two user
definable options, one for left click one for right click.
That way you dont have to remember modifiers.
So I can set up my go button to open in new window if I left click or in new tab
if I right click
And others can set their buttons to do what they want.

Comment 7

[Blake Ross]

Joe was dying to have these bugs. Who am I to say no?

Comment 8

[Caspy7]

I don't know about a modifier key for the go button, but as for prefs for I give
it a solid nay.  We're drowning in prefs as it is and anything that would modify
the left click on the go button would be highly suspect usability-wise.

My suggestion for the go button, and perhaps this should be another bug, would
be that:
- Left clicking gives standard behavior - "Go to the URL"
- Middle clicking opens the URL in a new window - like middle clicking a link. 
(I only have windows, don't know about alternate OS modifiers)
- Right clicking provides a context menu:
	Open in New Window
	Open in New Tab
	Save As...

Adding other items could be debated, but I think this would be sensible behavior.
Also if the URL is not new, but represents the current page, the middle and
right click could still apply to the current page.

Comment 9

[Jonas Jørgensen]

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

Comment 10

[Matthew T (active 1999-2002)]

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

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Taking bug, I have a patch

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

really taking this time

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

/me just spotted comment 5 and figures that the patch is not ready yet

removing dependency since that bug has been marked as dup of this one :)

Mark, please file a seperate bug for that, unless already filed.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

now also works when clicking the go button + pressing a modifier

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v3

do the right thing on macs too (ie. use Option (Alt) to save the file)


Ah yeah, one more thing, note that none of these three allow you to open a new
window in the background. I'll file a seperate bug for this.

Comment 17

[andreww]

Adding Aaron  Leventhal for accessibility comments. Aaron, does this 
conflict  in any way with anything via accessibility ?

Comment 18

[Aaron Leventhal]

No, I think this would be great.

Open in new tab or window would be very useful from there.
Personally, I think Search would be more useful from there than "save link as".

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

aaronl, well, this bug is for making modifier+enter behave the same as 
modifier+click on link, so that would be contrary to this bug's intention.

Also, I think that would be pretty unexpected for users. And there's always the 
"search" entry in the dropdown, easily reachable with up arrow+enter.

Shift+Enter makes it possible to paste a link into the location bar and easily 
save it.

Comment 20

[andreww]

I'll test it today

Comment 21

[andreww]

tested this on mac trunk from a few days ago. items 2 and 3 do not work 
(tried both the "return" key and the "enter" key)  4 and 5 did work.

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v4

ok, command==meta on mac...

AndrewW, could you try again with this patch please?

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v5

add some comments, don't reuse saveModifier for a different purpose, add small
fix for another problem

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

I didn't intend to comment this line out:
+          //newWin.blur();

for r=/sr= treat it as not commented out :)

Comment 25

[Scott Collins]

I applied the most recent patch to an OS X mach-o tree that is otherwise exactly
as the 1.1b release, and tested the result.  All worked as specified in the
(desired behavior section of the) initial comment except

  (3) the new window opened in _front_, just as with (2); and
  (5) a file-save-picker popped up, exactly as in (4)

Comment 26

[Kathleen :Brade]

Comment on attachment 92392 [details] [diff] [review]
patch v5

r=brade
thanks for adding the comments; I'd be even happier with more but what's there
is fine

Comment 27

[Alec Flett]

Comment on attachment 92392 [details] [diff] [review]
patch v5

sr=alecf

Comment 28

[Christian :Biesinger (don't email me, ping me on IRC)]

1.1b is out...

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed

Comment 30

[Jonas Jørgensen]

See also:

Bug 162193, "Ctrl+Enter in URL bar should load URL in a new window"

Bug 162189, "Ctrl+Enter in URL bar ignores "Load links in the background" pref".

Comment 31

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 32

[John Morrison]

This broke the following action in today's trunk win32 build (possibly others):
 1) have some url's in your urlbar history
 2) open the urlbar history popup
 3) click on one of the url's in the popup
Actual: nothing happens
Expected: browser loads url

There is a JS error about invalid something or other for 'in' operator. (Sorry, 
I can't see it offhand in the commercial build). Although I don't really get 
how it even reached that point in the code, given the preceding check for true.

Comment 33

[John Morrison]

Attachment: patch: group the or clause

Comment 34

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 95021 [details] [diff] [review]
patch: group the or clause

er, oops. that's what I really meant... sorry...

r=biesi

Comment 35

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 95021 [details] [diff] [review]
patch: group the or clause

sr=bzbarsky

Comment 36

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in regression fix

Comment 37

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 38

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 39

[Wesha]

v.