Bug 1302815 - Switch Firefox iOS to use list.json #3555
Conversation
SwiftLint found issuesWarnings
Generated by 🚫 Danger |
farhanpatel
force-pushed
the
1302815_list_json
branch
from
364f478
to
ce8f298
Compare
| // } | ||
| // | ||
| // Telemetry.default.add(pingBuilderType: CorePingBuilder.self) | ||
| // Telemetry.default.add(pingBuilderType: MobileEventPingBuilder.self) |
There was a problem hiding this comment.
woops. My bad. This commit is so annoyingly large that I didnt see this 😢
| } | ||
| return possibilities |
There was a problem hiding this comment.
This code is cleaner than what went before, to be sure.
// These exceptions can go away when we drop iOS 8 or when we start using a better mechanism for search
// engine selection that is not based on language identifier.What happens when we drop iOS8? What's in iOS9?
There was a problem hiding this comment.
Apple changed Locale Identifiers
|
|
||
| // might not work to change the default. | ||
| // if someone has added/removed an engine new engines or edits to the default engines might never be shown. |
| // setup the list json | ||
| let searchPrefs = DefaultSearchPrefs(with: Bundle.main.resourceURL!.appendingPathComponent("SearchPlugins").appendingPathComponent("list.json"))! | ||
|
|
||
| // setup the most popular locales |
There was a problem hiding this comment.
How do we know we're not shipping broken XML files to Indonesia or India?
For that matter, how do we know we're not shipping broken list.json? (smoke testing aside)
There was a problem hiding this comment.
The good thing about this setup vs the old one is. The XML files are all shared between countries. So if we test the global xml file in a test for en_CA then the XML file should also work in India.
As for List.json, Hopefully we arent merging patches to master without CI 😄
| <Param name="tag" value="firefox-fr-21"/> | ||
| <Param name="sourceid" value="Mozilla-search"/> | ||
| </Url> | ||
| <SearchForm>https://www.amazon.fr/</SearchForm> |
There was a problem hiding this comment.
Are these files actually changed, or are they simply mved? Did you git mv them?
There was a problem hiding this comment.
I picked up this patch from mkaply, according to him. These were just moved
farhanpatel
force-pushed
the
1302815_list_json
branch
from
ce8f298
to
24c0b25
Compare
This PR looks big but its mostly search metadata files being moved.
You can look at
DefaultSearchPrefs.swiftandSearchEngines.swiftfor the code changes.