Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab

VERIFIED FIXED in Firefox 51

Status

()

P2
normal
VERIFIED FIXED
3 years ago
7 months ago

People

(Reporter: Oriol, Assigned: adw)

Tracking

({regression})

51 Branch
Firefox 52
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(firefox48 unaffected, firefox49 unaffected, firefox50 unaffected, firefox51 verified, firefox52 verified)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
1. Go to http://example.com
2. Without closing that tab, open a new tab and start typing "example.com"
3. Keep Ctrl key pressed and click the entry with "Switch to Tab"

The new tab is closed, and the old one is focused.

I expected loading http://example.com in a new tab because of Ctrl key.
status-firefox48: --- → unaffected
status-firefox49: --- → unaffected
status-firefox50: --- → unaffected
(Assignee)

Updated

3 years ago
Assignee: nobody → adw
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [fxsearch]
Comment hidden (mozreview-request)
(Assignee)

Comment 2

2 years ago
Thanks for filing this.

Marco, I was about to reply that I was confused because Ctrl doesn't actually affect switchtab results, but lo and behold it actually does -- or did before the urlbar one-offs bug changed the URL load path in urlbar.  The reason I thought that is because Ctrl isn't one of the noactions keys.  But the way the URL-loading path used to work, that didn't matter.  It called whereToOpenLink for mouse events and then called openUILinkIn directly when where != "current", bypassing switchtab (and all mozactions I guess?): https://hg.mozilla.org/mozilla-central/annotate/56fc33e6b14e/browser/base/content/urlbarBindings.xml#l433

At first I thought OK, I'll just modify urlbar's handleCommand to break out of the switchtab case when where != "current".  But I think we should treat Ctrl as an official noactions key, so that the UI is updated when you press it, etc., even if it was unintended (?) that it was treated as an unofficial noactions key before.

Comment 3

2 years ago
mozreview-review
Comment on attachment 8801461 [details]
Bug 1296366 - Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab.

https://reviewboard.mozilla.org/r/86212/#review85560

My only fear is that with this simple override we have eaten all the available modifier keys, if in future we need a different kind of override, it will be fun.
But looks like some users were used to CTRL and for now we don't have a good enough reason to break that.

::: browser/base/content/urlbarBindings.xml:990
(Diff revision 1)
>  
>        <field name="_noActionsKeys"><![CDATA[
>          new Set();
>        ]]></field>
>  
> +      <field name="_possibleNoActionsKeys"><![CDATA[

_noActionsKeys is quite confusing with this.

I'd suggest to instead name this _noActionKeys and rename _noActionsKeys to _pressedNoActionKeys.

So we also remove some confusion about "action" vs "actions", that sounds like a good recipe for typos.
Attachment #8801461 - Flags: review?(mak77) → review+
Comment hidden (mozreview-request)

Comment 5

2 years ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dda934940e4d
Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab. r=mak

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dda934940e4d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
(Assignee)

Comment 7

2 years ago
Created attachment 8802646 [details] [diff] [review]
Aurora 51 patch

Approval Request Comment
[Feature/regressing bug #]: One-off searches in the urlbar, bug 1180944
[User impact if declined]: Ctrl-clicking switch-to-tab results in the urlbar will not open them in new tabs
[Describe test coverage new/current, TreeHerder]: Manual testing
[Risks and why]: Low risk, small patch
[String/UUID change made/needed]: None
Attachment #8802646 - Flags: approval-mozilla-aurora?
Comment on attachment 8802646 [details] [diff] [review]
Aurora 51 patch

Fix a regression related to awesomebar. Take it in 51 aurora.
Attachment #8802646 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 9

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/acaf3a268d2f
status-firefox51: affected → fixed

Comment 10

2 years ago
This is fixed only on Mac OS X. 

It's still broken on Windows and Ubuntu (I tested Windows 10 and Ubuntu 16.04) on both Nightly 52 (Build ID  20161024030205) and Aurora 51 (Build ID  20161024004016).

Also on Mac, even if it works, when pressing "Command" key, the "Switch to tab" text from URL is replaced by the address of the webpage (e.g. "http://example.com"). Is this intended?
Flags: needinfo?(adw)
(Assignee)

Updated

2 years ago
Status: RESOLVED → REOPENED
Flags: needinfo?(adw)
Resolution: FIXED → ---
(Assignee)

Updated

2 years ago
Blocks: 1313969
(Assignee)

Comment 11

2 years ago
I opened follow-up bug 1313969 because mozreview is awful.  Maybe that's better for tracking anyway, dunno.  I guess we should leave this one open until the new one is resolved, not sure.

Thanks very much Liviu for catching this.

(In reply to Liviu Cirdei [:liviucirdei] from comment #10)
> Also on Mac, even if it works, when pressing "Command" key, the "Switch to
> tab" text from URL is replaced by the address of the webpage (e.g.
> "http://example.com"). Is this intended?

"Switch to tab" should be removed from the urlbar, and the URL should occupy the entire urlbar.  Is that what you're seeing?  It's the same thing that should happen when you press Alt and Shift too.

Comment 12

2 years ago
(In reply to Drew Willcoxon :adw from comment #11)

> "Switch to tab" should be removed from the urlbar, and the URL should occupy
> the entire urlbar.  Is that what you're seeing?  It's the same thing that
> should happen when you press Alt and Shift too.

This is what I'm seeing: http://imgur.com/a/TLrDU. And is the same thing when I press Alt or Shift.
(Assignee)

Comment 13

2 years ago
Thanks for the screenshot.  Yes, that's the intended behavior because you're overriding the usual switch-to-tab behavior.
(Assignee)

Comment 14

2 years ago
The follow-up bug has been fixed, so I'll mark this as fixed again too.
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED

Comment 15

2 years ago
This was verified in the follow-up bug 1313969, so I'm changing the status of this one to verified.
Status: RESOLVED → VERIFIED
status-firefox51: fixed → verified
status-firefox52: fixed → verified
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.