Review SearchService.getEngineParams, move into SearchEngine
Categories
(Firefox :: Search, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox80 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(3 files, 3 obsolete files)
SearchService.getEngineParams is currently listed on the idl although it is not used outside the search service:
https://searchfox.org/mozilla-central/search?q=getEngineParams&path=&case=false®exp=false
We should definitely make it non-public.
Additionally, we should review its uses. AFAICT it is only ever used before a call to SearchEngine._initFromMetadata or SearchEngine._updateFromMetadata.
The function of getEngineParams is to take the a WebExtension's search engine details and translate them into a different format for the *FromMetadata functions.
I think we should just merge getEngineParams across into _initFromMetadata and avoid the overhead and complication of the extra translation.
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 1•4 years ago
|
||
The clones appear to be so that access for the tree view in preferences is quick. However, they don't need to clone the entire engine object - they just need three fields.
Additionally, this fixes reloading icons which was attempting to use 'uri' but that isn't defined, and so icons would fail to load if preferences was opened when a search engine is added.
Depends on D80472
|
|
Assignee | |
Comment 2•4 years ago
|
||
Depends on D80496
|
|
Assignee | |
Comment 3•4 years ago
|
||
Depends on D80497
|
||
Comment 4•4 years ago
|
||
Comment on attachment 9158245 [details]
Bug 1637744 - Preferences should only clone the parts of the search engine objects that it actually needs. r?jaws!
Revision D80496 was moved to bug 1647359. Setting attachment 9158245 [details] to obsolete.
|
|
||
Comment 5•4 years ago
|
||
Comment on attachment 9158246 [details]
Bug 1637744 - Change definitions in SearchEngine to be classes. r?daleharvey!
Revision D80497 was moved to bug 1647359. Setting attachment 9158246 [details] to obsolete.
|
|
||
Comment 6•4 years ago
|
||
Comment on attachment 9158247 [details]
Bug 1637744 - Move SearchEngine fields to start of class for better clarity and documentation. r?daleharvey!
Revision D80498 was moved to bug 1647359. Setting attachment 9158247 [details] to obsolete.
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 7•4 years ago
|
||
The main aim here is to move the call to getEngineParams that currently happens before addEngineWithDetails. This is moved into addEngineWithDetails, so that it is next to where the search engine is actually created. This gets ready for the next step which will be to merge getEngineParams with the SearchEngine initWithMetadata and associated calls.
The side effects are that we need a specific function for policy engines to use, and that we now have only tests using addEngineWithDetails.
|
|
Assignee | |
Comment 8•4 years ago
|
||
These cover some gaps in coverage for the existing getEngineParams.
Depends on D82348
|
|
Assignee | |
Comment 9•4 years ago
|
||
Depends on D82349
|
|
Assignee | |
Comment 10•4 years ago
|
||
|
|
Assignee | |
Comment 11•4 years ago
|
||
|
||
Comment 12•4 years ago
|
||
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4a3af68e0ae1 Create a new function for adding search engines, and a separate function for policy engines. r=daleharvey https://hg.mozilla.org/integration/autoland/rev/3a9439b6ba13 Add more tests to cover various search engine parameter options. r=daleharvey https://hg.mozilla.org/integration/autoland/rev/a9a5f77ddc68 Merge getEngineParams into functions in SearchEngine. r=daleharvey
|
||
Comment 13•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4a3af68e0ae1
https://hg.mozilla.org/mozilla-central/rev/3a9439b6ba13
https://hg.mozilla.org/mozilla-central/rev/a9a5f77ddc68
Description
•