Add additional engines for testing

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Firefox 58
Points:
---

Firefox Tracking Flags

(firefox57blocking fixed, firefox58 fixed)

Details

Attachments

(2 attachments)

We're going to be doing some tests soon, so we want to prebundle some engines for later testing.

These engines will be going on desktop and mobile.
Comment on attachment 8927577 [details]
Bug 1416484 - Add and update some engines for later testing.

https://reviewboard.mozilla.org/r/198876/#review203912

This looks pretty good.  Everything is disabled, so it should be safe to uplift.  At minimum we should get it into Fx58.  I wonder if we can get this into 57 (basically zero risk, we've done this before (bug 1229667 and bug 1339643) without ill effects ?  Is there going to be an RC3?
Attachment #8927577 - Flags: review?(mconnor) → review+
Please land that in mi and mc asap. also fill the uplift request so that we are ready.
Marking it as blocking as it is partner related. 
We will do a RC 3 to fix an issue with shield experiments. Probably go to build tomorrow 
Next time, please let us know earlier for last minute changes
Assignee: nobody → mozilla
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/7974ca5efe2b
Add and update some engines for later testing. r=mconnor
https://hg.mozilla.org/mozilla-central/rev/7974ca5efe2b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8927577 [details]
Bug 1416484 - Add and update some engines for later testing.

Approval Request Comment

[Feature/Bug causing the regression]: n/a

[User impact if declined]:
no user impact, impacts our ability to do some search testing

[Is this code covered by automated tests?]: yes

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: 
none at this time., QA will be done as a part of any test rollout

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: no

[Why is the change risky/not risky?]: We've landed multiple hidden & experimental plugins over the years, as noted above, without any known regressions.  This change does not modify the search engines that will be used for any users. Prior to any search test, we will conduct additional QA to ensure everything keeps working as intended.

[String changes made/needed]: none
Attachment #8927577 - Flags: approval-mozilla-release?
Attachment #8927577 - Flags: approval-mozilla-beta?
Comment on attachment 8927577 [details]
Bug 1416484 - Add and update some engines for later testing.

https://reviewboard.mozilla.org/r/198876/#review203930

::: browser/locales/searchplugins/amazon-nl.xml:6
(Diff revision 2)
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
> +<ShortName>Amazon.com.nl</ShortName>

This should be Amazon.nl
Comment on attachment 8927577 [details]
Bug 1416484 - Add and update some engines for later testing.

No need to take it to beta, it will be automagically applied during the last m-c => m-b merge.
Attachment #8927577 - Flags: approval-mozilla-release?
Attachment #8927577 - Flags: approval-mozilla-release+
Attachment #8927577 - Flags: approval-mozilla-beta?
Landed on m-r:
https://hg.mozilla.org/releases/mozilla-release/rev/a5a689f01f0ce0dc4b61711bfc7f992e666bf7d9

Flod, about comment #8, I ask for a confirmation that it is indeed a typo (as amazon.com.nl redirects). I will fix that in case it is.
(In reply to Sylvestre Ledru [:sylvestre] from comment #10)
> Landed on m-r:
> https://hg.mozilla.org/releases/mozilla-release/rev/
> a5a689f01f0ce0dc4b61711bfc7f992e666bf7d9
> 
> Flod, about comment #8, I ask for a confirmation that it is indeed a typo
> (as amazon.com.nl redirects). I will fix that in case it is.

NI for any of the two Mike, whoever comes first.

It looks like bad copy and paste from one of the others Amazon xml files (com.au, com.br). 

The mobile one uses Amazon.nl
https://hg.mozilla.org/releases/mozilla-release/rev/a5a689f01f0ce0dc4b61711bfc7f992e666bf7d9#l14.10

In case, both ShortName and Description should be Amazon.nl, since that's the domain used for searching
https://hg.mozilla.org/releases/mozilla-release/rev/a5a689f01f0ce0dc4b61711bfc7f992e666bf7d9#l6.10
Flags: needinfo?(mozilla)
Flags: needinfo?(mconnor)
It's a typo, please fix. (that's not a url, it's the display string)

There's no functionality issue, just visual.
(In reply to Mike Connor [:mconnor] from comment #12)
> It's a typo, please fix. (that's not a url, it's the display string)

To summarize: in browser/locales/searchplugins/amazon-nl.xml, both ShortName and Description should read Amazon.nl instead of Amazon.com.nl
Flags: needinfo?(sledru)
Flags: needinfo?(mozilla)
Flags: needinfo?(mconnor)
Posted patch am.diffSplinter Review
Flags: needinfo?(sledru)
Attachment #8927643 - Flags: review?(mconnor)
Attachment #8927643 - Flags: review?(francesco.lodolo)
Attachment #8927643 - Flags: review?(francesco.lodolo) → review+
Attachment #8927643 - Flags: review?(mconnor)
You need to log in before you can comment on or make changes to this bug.