1391239 - pressing AltGr-Enter in the search bar should open page in a new tab

Status: RESOLVED FIXED

(Firefox :: Search, defect, P2)

Description

[Claude Gohier]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170816100153

Steps to reproduce:

1. Enter URL in address bar, or search terms in search bar.
2. Press AtlGr-Enter


Actual results:

Page is loaded in current tab.


Expected results:

Page is loaded in new tab, the way it works in Windows.

Comment 1

[Claude Gohier]

I'm using the French (Canada) keyboard layout.

Comment 2

[Marco Bonardo [:mak]]

The problem is that ALT+GR is interpreted as CTRL+ALT.

Comment 3

[Claude Gohier]

Actually, no. AltGr-Enter does not work like CTRL-ALT-Enter on Linux.

ALT-CTRL-ENTER is canonizing and opening in a new tab.
AltGr-ENTER does the same thing as ENTER alone.

Comment 4

[Marco Bonardo [:mak]]

(In reply to Claude Gohier from comment #3)
> Actually, no. AltGr-Enter does not work like CTRL-ALT-Enter on Linux.

Yeah no, not on Linux, it does on Windows.

Comment 5

[Henrik Pauli]

I'd say Linux is doing it right (AltGr shouldn't be interpreted as ctrl or alt or both), and Windows is in the wrong by emulating some pre-PC-AT behaviour by sending a Ctrl+Alt combo instead of anything sensible.  If anything, the inverse of this bug should be opened and this closed as invalid.  Actually, see Bug 900750.

Comment 6

[Claude Gohier]

I'd say the AltGr-Enter combo on Linux is useless, since it just does the same thing as only Enter.

The Windows comportment, having AltGr-Enter doing the same thing as Alt-Enter (loading in a new tab) allows to do a one-handed movement, which can also be considered to be an accessibility issue.

Comment 7

[Claude Gohier]

The fix for bug 1389739 seems to have fix this bug.

Comment 8

[Claude Gohier]

Fixed for the Address bar, but not the Search bar.

Comment 9

[Eugene Savitsky]

Bhopesh Bassi, could you help, please?

Comment 10

[Bhopesh Bassi]

I can help if someone experienced gives me good pointers on how to fix this. Pointers in https://bugzilla.mozilla.org/show_bug.cgi?id=1389739 helped me fix it quickly.

Comment 11

[Eugene Savitsky]

Drew Willcoxon, could you please point Bhopesh where to fix this?

I assume it should be smth like this (based on bug 1389739):

https://searchfox.org/mozilla-central/rev/40e889be8ff926e32f7567957f4c316f14f6fbef/browser/components/searchbar/SearchbarInput.jsm#1282

To this:

if (
!isMouseEvent &&
event &&
(event.altKey || event.getModifierState("AltGraph"))
) {

I wish we could get it done for FF70 beta.

Comment 12

[Eugene Savitsky]

https://searchfox.org/mozilla-central/source/browser/components/search/content/searchbar.js

Isn't it?

Comment 13

[Bhopesh Bassi]

I will take a look at it.

Comment 14

[Drew Willcoxon :adw]

Yes, this method specifically: https://searchfox.org/mozilla-central/rev/428cf94f4ddfb80eebc6028023a7d89eb277791b/browser/components/search/content/searchbar.js#270

Comment 15

[Eugene Savitsky]

Any progress? Soon the v70 train will depart... :(

Comment 16

[Bhopesh Bassi]

When does v70 train leave? I have been busy at my day job lately. I will work on it soon.

Comment 17

[Bhopesh Bassi]

Hey Drew, Can you share test cases files as well? Thanks

Comment 18

[Eugene Savitsky]

For v70 it is too late (first beta released), if only they do not take the fix in beta cycle in.

Comment 19

[Drew Willcoxon :adw]

It looks like this is the only test that checks enter key handling in the search bar: https://searchfox.org/mozilla-central/source/browser/components/search/test/browser/browser_426329.js

Comment 20

[Eugene Savitsky]

Bhopesh Bassi, no hope for a fix?

Comment 21

[Bhopesh Bassi]

I am really sorry Eugene. I have been extremely busy at my new job. I will try to finish it by October end.

Again, I sincerely apologize that it is taking so much time.

Comment 22

[Bhopesh Bassi]

Attachment: Bug 1391239 - Make AltGr-Enter in the search bar open a new tab.

Comment 23

[Bhopesh Bassi]

Attachment: Fix Bug 1391239.

Depends on D51504

Comment 24

[Bhopesh Bassi]

I have fixed the bug. Accidentally I created two commits(and hence two revisions) by mistake. Do I need to merge these or fix these in some other way?

Here are the links to phabricator revisions. I have added Drew as reviewer on both.
https://phabricator.services.mozilla.com/D51504
https://phabricator.services.mozilla.com/D51505

Comment 25

[Drew Willcoxon :adw]

Hi, sorry for the delay. Yes, please merge your commits. Our workflow is that we have one revision per reviewable chunk of code, and this case is certainly simple enough for one revision. (If a bug is big and requires many different loosely coupled parts, then we'll often split that up into multiple revisions.) Depending on your personal workflow, you can make as many commits locally as you like, but when requesting review, and when posting new versions of your work due to review comments, please squash them somehow and post a single revision. In this case, it looks like you can merge your D51505 into D51504 and then send D51504 to Phabricator again.

Comment 26

[Eugene Savitsky]

Any update? FF72 beta cycle is near the end...

Comment 27

[Mark Banner (:standard8)]

Please don't keep nagging. It isn't polite and is against our etiquette.

Comment 28

[Bhopesh Bassi]

Hey Drew any updates?

Comment 29

[Drew Willcoxon :adw]

Bhopesh, Eugene, I'm very sorry I haven't been responsive on this bug. I don't blame you for asking about the status. That's perfectly reasonable, and I would have been asking a lot more if I were you.

I just queued Bhopesh's patch for landing, so it should make 72. The bug will be updated first when the patch lands on autoland and then again when it lands on mozilla-central. It will be closed once it's on mozilla-central.

Thanks Bhopesh for fixing this and the other bug.

Comment 30

[Bhopesh Bassi]

Thanks a lot Drew. I don't blame Eugene for asking for status. Infact, I think I should have been more prompt to fix this bug.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc66012d716d
Make AltGr-Enter in the search bar open a new tab. r=adw

Comment 32

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/dc66012d716d

Comment 33

[Eugene Savitsky]

Guys, thanks a lot! I will donate 25 € to Mozilla! It seems I will be happy again soon! :)

Comment 34

[Bhopesh Bassi]

You are welcome :-)