Closed Bug 1486819 Opened Last year Closed Last year

add support for missing internal features for search engines

Categories

(WebExtensions :: General, enhancement, P1)

enhancement

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

Blocks: 1486820
Assignee: nobody → mixedpuppy
Priority: -- → P1
Duplicate of this bug: 1486818
Depends on: 1488517
Expanding this to include moz:* parameters and the pref capability.
Summary: add mozparam support for search engines → add support for missing internal features for search engines
    "name": "MozParamsTest",
    "search_url": "https://example.com/?q={searchTerms}",
    "params": [...mozParams, ...params],


Should this be "search_url_get_params" with a corresponding one for "suggest_url"? I havent found a case in which we use MozParams for suggest_url, so we could just encode the params in the url fields for plain 'Params' and use the params field only for MozParams. But that seems a little confusing
It seems like we dont support "search_url_post_params" to start with[1], we have some built in engines using POSTs @ https://searchfox.org/mozilla-central/source/browser/components/search/searchplugins/freelang.xml, will this need fixed or am I missing a different way to do these

[1] - https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/schemas/chrome_settings_overrides.json#69
Depends on: 1485508
No longer depends on: 1488517
(In reply to Dale Harvey (:daleharvey) from comment #4)
>     "name": "MozParamsTest",
>     "search_url": "https://example.com/?q={searchTerms}",
>     "params": [...mozParams, ...params],
> 
> 
> Should this be "search_url_get_params" with a corresponding one for
> "suggest_url"? I havent found a case in which we use MozParams for
> suggest_url, so we could just encode the params in the url fields for plain
> 'Params' and use the params field only for MozParams. But that seems a
> little confusing

These are not get params, or rather, they will work with either get or post.  I'll think about getting rid of the plain param as you suggested.
Comment on attachment 9004646 [details]
Bug 1486819 - support mozParams in webext search engines

Andrew Swan [:aswan] has approved the revision.
Attachment #9004646 - Flags: review+
Comment on attachment 9004646 [details]
Bug 1486819 - support mozParams in webext search engines

Drew Willcoxon :adw has approved the revision.
Attachment #9004646 - Flags: review+
refactor some code into the search service.  This is necessary to
allow the searchservice to pull multiple locales or regions from a single
extension, based on data the searchservice maintains.
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1f268b2058c9
support mozParams in webext search engines r=aswan,adw,mkaply
https://hg.mozilla.org/integration/autoland/rev/47f1126313c6
move search engine specific logic into search service, r=mkaply
https://hg.mozilla.org/mozilla-central/rev/1f268b2058c9
https://hg.mozilla.org/mozilla-central/rev/47f1126313c6
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Keywords: dev-doc-needed
for dev-doc, specifically the suggest_url, suggest_url_post_params is now supported.  search_url_post_params was added in another bug as well.

I'm unsure if we want to document the param support on mdn or not as that is purely internal use, and only with builtin search extensions.
correction, the condition/purpose and pref params are internal only.  There are other params that are supported which map to some opensearch functionality and you can see that in the test file, but here they are:

+    {name: "simple", value: "5"},
+    {name: "term", value: "{searchTerms}"},
+    {name: "lang", value: "{language}"},
+    {name: "locale", value: "{moz:locale}"},
Will this fix require manual validation? if so please provide some steps to help validate it or if it won't be necessary please set the flag 'qe-validate-'. Thank you
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(mixedpuppy) → qe-verify-
Note to documentation team:

I've included a note in the Fx64 rel notes to cover this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#Changes_for_add-on_developers

Feel free to update this if you think it needs fixing up.

It looks like they are mentioned in the docs already too, but you should still check it out, and make sure browser compat data is updated, etc.
Updated BCD and created a pull request: https://github.com/mdn/browser-compat-data/pull/3089
Blocks: 1517486
You need to log in before you can comment on or make changes to this bug.