Closed Bug 1296366 Opened 8 years ago Closed 8 years ago

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

Categories

(Firefox :: Search, defect, P2)

51 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 52
Tracking Status
firefox48 --- unaffected
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- verified
firefox52 --- verified

People

(Reporter: Oriol, Assigned: adw)

References

Details

(Keywords: regression, Whiteboard: [fxsearch])

Attachments

(2 files)

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.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [fxsearch]
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/dda934940e4d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Attached patch Aurora 51 patchSplinter Review
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+
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)
Status: RESOLVED → REOPENED
Flags: needinfo?(adw)
Resolution: FIXED → ---
Blocks: 1313969
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.
(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.
Thanks for the screenshot.  Yes, that's the intended behavior because you're overriding the usual switch-to-tab behavior.
The follow-up bug has been fixed, so I'll mark this as fixed again too.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
This was verified in the follow-up bug 1313969, so I'm changing the status of this one to verified.
Status: RESOLVED → VERIFIED
Flags: in-qa-testsuite+
See Also: → 1536756
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: