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)
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)
(deleted),
text/x-review-board-request
|
mak
:
review+
|
Details |
(deleted),
patch
|
gchang
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•8 years ago
|
status-firefox48:
--- → unaffected
status-firefox49:
--- → unaffected
status-firefox50:
--- → unaffected
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → adw
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [fxsearch]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•8 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•8 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) |
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•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Assignee | ||
Comment 7•8 years ago
|
||
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•8 years 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•8 years ago
|
||
bugherder uplift |
Comment 10•8 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•8 years ago
|
Status: RESOLVED → REOPENED
Flags: needinfo?(adw)
Resolution: FIXED → ---
Assignee | ||
Comment 11•8 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•8 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•8 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•8 years ago
|
||
The follow-up bug has been fixed, so I'll mark this as fixed again too.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Comment 15•8 years ago
|
||
This was verified in the follow-up bug 1313969, so I'm changing the status of this one to verified.
Updated•6 years ago
|
Flags: in-qa-testsuite+
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•