1480504 - Add @amazon and @google as internal search keywords

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[Tim Spurway [:tspurway]]

We can do this by adding additional 'aliasMatches', which would allow us to add our keywords and would not collide with user defined search keywords.  

The one edge case will will still need to take care of here would be if the user removes the actual search provider.

Comment 1

[Justin Dolske [:Dolske]]

Needs implemented by the fxsearch team, not AS.

Comment 2

[Mike Kaply [:mkaply]]

> The one edge case will will still need to take care of here would be if the user removes the actual search provider.

The user can't remove them, they can only hide them, and I think the aliases should still work in that case.

Comment 3

[Ed Lee :Mardak]

(In reply to Mike Kaply [:mkaply] from comment #2)
> The user can't remove them, they can only hide them
Aha! That would simplify things.

Any suggestions for a good way to convert existing search plugins to have the internal alias? We probably only want to do this for default/packaged plugins as potentially there could be alias conflicts if someone installed 2+ amazon search engines.

Comment 4

[Mike Kaply [:mkaply]]

We can know if a search engine is built in, so we can check for that specifically. And there can't be another engine named Google.

I think what we should do is setup Google as is (since it will just work).

For Amazon, loop through all the engines, look for a built in engine whose name begins with "Amazon" and set the alias to that one. 

I don't mind coding this up really quick. I had already been messing around with this.

Comment 5

[Mike Kaply [:mkaply]]

Attachment: First pass

Here's a quick first pass.

I didn't reuse _addEngine because we don't need the priorityMatches code.

I verified this works even when Google or Amazon is hidden.

If we wanted to make this more generic, we could have a mapping of some sort:

[
"Google": "@google",
"Amazon": "@amazon",
"eBay": "@ebay",
]

and then just match the first part of the left side against a search engine and add the right side as an alias.

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Bug 1480504 - Add internal aliases for common engines.

Comment 7

[Mike Kaply [:mkaply]]

I just pushed a more clever version that we can easily add new engines to.

There's probably a better way to create the initial map, but I couldn't thnk of anything in the moment.

It matches against the beginning of the engine name would should cover us for most cases.

Comment 8

[Ed Lee :Mardak]

Looks like the approach here should also fix bug 1480509 where this bug was made for existing profiles and the other bug is for a new profile, yeah?

Comment 9

[Mike Kaply [:mkaply]]

This adds the internal shortcuts, but I think bug 1480509 is about adding the actual tiles, right?

Comment 11

[Phabricator Automation]

Comment on attachment 8997157 [details]
Bug 1480504 - Add internal aliases for common engines.

Ed Lee :Mardak has approved the revision.

https://phabricator.services.mozilla.com/D2681

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ab02d99fe7e
Add internal aliases for common engines. r=Mardak

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2ab02d99fe7e

Comment 14

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-beta/rev/a9f598988ff938aef7526d15b532ec2c91806f77

Comment 15

[Cristian Fogel [:cfogel]]

There are some extra scenarios that could affect this feature: 
 
- if a user wants to search for the "@amazon" or "@google" strings is there any way to cancel it out? *the user would have to open the search engine and re-input the strings;
- with "@amazon" or "@google" in the awesome-bar pressing CTRL+ENTER brings up the  log in to the site(with username) confirmation window;

Do we consider any of these 2 blockers for the current implementation or should we treat them in separate threads/bugs?

Comment 16

[Cristian Fogel [:cfogel]]

Verified on MacOS 10.9, Ubuntu 16.04LTS, Win 10x64 for both 62.0b18 and 63.0a1 (2018-08-19).
Would address the issues in their own threads + the extra one.

Comment 17

[Mike Kaply [:mkaply]]

These issues are addressed in separate bugs.