pressing AltGr-Enter in the search bar should open page in a new tab
Categories
(Firefox :: Search, defect, P2)
Tracking
()
People
(Reporter: cgohier, Assigned: bbassi)
References
(Regressed 1 open bug)
Details
(Whiteboard: [fxsearch])
Attachments
(1 file, 1 obsolete file)
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.
Updated•7 years ago
|
Reporter | ||
Comment 1•7 years ago
|
||
I'm using the French (Canada) keyboard layout.
Updated•7 years ago
|
Comment 2•7 years ago
|
||
The problem is that ALT+GR is interpreted as CTRL+ALT.
Reporter | ||
Comment 3•7 years ago
|
||
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•7 years ago
|
||
(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.
Updated•7 years ago
|
Comment 5•7 years ago
|
||
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.
Reporter | ||
Comment 6•7 years ago
|
||
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.
Reporter | ||
Comment 7•5 years ago
|
||
The fix for bug 1389739 seems to have fix this bug.
Reporter | ||
Comment 8•5 years ago
|
||
Fixed for the Address bar, but not the Search bar.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 10•5 years ago
|
||
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•5 years ago
|
||
Drew Willcoxon, could you please point Bhopesh where to fix this?
I assume it should be smth like this (based on bug 1389739):
To this:
if (
!isMouseEvent &&
event &&
(event.altKey || event.getModifierState("AltGraph"))
) {
I wish we could get it done for FF70 beta.
Comment 12•5 years ago
|
||
Assignee | ||
Comment 13•5 years ago
|
||
I will take a look at it.
Comment 14•5 years ago
|
||
Yes, this method specifically: https://searchfox.org/mozilla-central/rev/428cf94f4ddfb80eebc6028023a7d89eb277791b/browser/components/search/content/searchbar.js#270
Comment hidden (advocacy) |
Assignee | ||
Comment 16•5 years ago
|
||
When does v70 train leave? I have been busy at my day job lately. I will work on it soon.
Assignee | ||
Comment 17•5 years ago
|
||
Hey Drew, Can you share test cases files as well? Thanks
Comment hidden (advocacy) |
Comment 19•5 years ago
|
||
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 hidden (advocacy) |
Assignee | ||
Comment 21•5 years ago
|
||
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.
Assignee | ||
Comment 22•5 years ago
|
||
Assignee | ||
Comment 23•5 years ago
|
||
Depends on D51504
Assignee | ||
Comment 24•5 years ago
|
||
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•5 years ago
•
|
||
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.
Updated•5 years ago
|
Comment hidden (advocacy, me-too) |
Comment hidden (off-topic) |
Updated•5 years ago
|
Updated•5 years ago
|
Comment 29•5 years ago
|
||
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.
Assignee | ||
Comment 30•5 years ago
|
||
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•5 years ago
|
||
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dc66012d716d Make AltGr-Enter in the search bar open a new tab. r=adw
Comment 32•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 33•5 years ago
|
||
Guys, thanks a lot! I will donate 25 € to Mozilla! It seems I will be happy again soon! :)
Assignee | ||
Comment 34•5 years ago
|
||
You are welcome :-)
Description
•