|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
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.
11 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 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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/dda934940e4d Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab. r=mak
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
Comment on attachment 8802646 [details] [diff] [review] Aurora 51 patch Fix a regression related to awesomebar. Take it in 51 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?
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.
This was verified in the follow-up bug 1313969, so I'm changing the status of this one to verified.