1313969 - [Follow-up] Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab

Status: VERIFIED FIXED

(Firefox :: Search, defect, P2)

Description

[Drew Willcoxon :adw]

+++ This bug was initially created as a clone of Bug #1296366 +++

Can't push a review request to reviewboard for a bug that's already been closed, so I'm opening a new one.

(In reply to Liviu Cirdei [:liviucirdei] from comment #10)
> 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?

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1313969 - [Follow-up] Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab.

Review commit: https://reviewboard.mozilla.org/r/89508/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89508/

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8805894 [details]
Bug 1313969 - [Follow-up] Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab.

https://reviewboard.mozilla.org/r/89508/#review88842

::: browser/base/content/urlbarBindings.xml:1003
(Diff revision 1)
>  
>            return action;
>          ]]></body>
>        </method>
>  
> -      <field name="_noActionKeys"><![CDATA[
> +      <property name="_noActionKeys" readonly="true">

is there a specific reason to convert from field to a property?
Is it just to avoid paying the AppConstants price?

Comment 3

[Drew Willcoxon :adw]

The only reason is because now there's logic to it, it's not just a simple value.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8805894 [details]
Bug 1313969 - [Follow-up] Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab.

https://reviewboard.mozilla.org/r/89508/#review88922

well, ok. I honestly don't know if we have guidelines for field VS property, in this case the perf differences are uninteresting. Not that I care that much.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c42b82086c26
[Follow-up] Ctrl+Click awesomebar entry with "Switch to Tab" doesn't open new tab. r=mak

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/c42b82086c26

Comment 7

[Drew Willcoxon :adw]

Attachment: Aurora 51 patch

Approval Request Comment
[Feature/regressing bug #]:
One-off searches in awesomebar, bug 1180944.  This was already "fixed" and uplifted in bug 1296366, but the fix didn't actually work on Windows and Linux, only Mac.

[User impact if declined]:
Users on Windows and Linux will see the original bug, bug 1296366: ctrl+clicking on an awesomebar switch-to-tab result doesn't open it in a new tab.

[Describe test coverage new/current, TreeHerder]:
Manual testing

[Risks and why]:
Low risk, small patch

[String/UUID change made/needed]:
None

Comment 8

[Gerry Chang [:gchang]]

Comment on attachment 8806229 [details] [diff] [review]
Aurora 51 patch

Fix a regression related to awesomebar. Take it in 51 aurora.

Comment 9

[Liviu Cirdei]

The fix is verified on Nightly 52.0a1 (Build ID 20161101030207). Tested on:
* Windows 10
* Windows 7
* Ubuntu 15.04

Comment 10

[Drew Willcoxon :adw]

Thanks Liviu!

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3c4ab75a8bcb

Comment 12

[Liviu Cirdei]

The fix is verified on Aurora 51.0a2 (Build ID 20161102004003). I tested on:
*Windows 10
*Ubuntu 16.04
*Mac 10.11