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

VERIFIED FIXED in Firefox 51

Status

()

Firefox
Search
P2
normal
VERIFIED FIXED
9 months ago
7 months ago

People

(Reporter: Oriol, Assigned: adw)

Tracking

({regression})

51 Branch
Firefox 52
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [fxsearch])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

9 months 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.

Updated

9 months ago
status-firefox48: --- → unaffected
status-firefox49: --- → unaffected
status-firefox50: --- → unaffected
(Assignee)

Updated

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

Comment 2

8 months 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

7 months 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

7 months 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

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

Comment 7

7 months 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 8

7 months ago
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

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

Comment 10

7 months 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

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

Updated

7 months ago
Blocks: 1313969
(Assignee)

Comment 11

7 months 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

7 months 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

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

Comment 14

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

Comment 15

7 months 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
You need to log in before you can comment on or make changes to this bug.