Closed
Bug 1437942
Opened 6 years ago
Closed 6 years ago
Remove search engines from language packs
Categories
(Firefox :: Search, enhancement, P1)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 62
People
(Reporter: mkaply, Assigned: mkaply)
References
(Blocks 1 open bug)
Details
Attachments
(8 files, 1 obsolete file)
8.78 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-review-board-request
|
Details | |
59 bytes,
text/x-review-board-request
|
mkaply
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ted
:
review+
mkaply
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ted
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mkaply
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ted
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
adw
:
review+
|
Details |
The end goal for the search JSON work was to move to a place where we shipped all searcn engines in Firefox and did the right thing regardless of your language (Region focus). It's time to finally do that. We should package all engines in the base product. Language packs should be just language. This will mean moving to a model where the entire list.json file is packaged in Firefox and parsed at runtime (including regionOverrides). This was my original vision, but we just weren't there yet with multilocale Firefox. We're getting there now, so it's time to change.
Comment 1•6 years ago
|
||
From a build system perspective this sounds great. :) We've been working on moving things out of Makefile.in files into moz.build files, but I punted on the searchplugins stuff in bug 1407427 because we didn't have any moz.build construct that would work for it. If you want to fix it to not be localized instead that would also solve our problem! I'd be happy to advise or help out with any build system issues here.
Comment 2•6 years ago
|
||
For the build system I think we need a metal concept of "different list of locales to package" per data piece. For example, we want to build Firefox with the following language resources: ["en-US"] and the following searchplugins: [...]. In the future we may want to package, say, 4 locales for a Swiss Firefox and 100 searchplugins and 100 crashreporter/installer languages (because other languages can be installed via langpacks but won't cover installer/crashreporter). Haveing ability to decide what locales to package for what dataset separately would help us get to a multilocale builds. (In this case, instead of thinking of searchplugins as "package them all", I'd like us to think about it as the first example of a dataset which will get it's own list of locales to package)
Comment 3•6 years ago
|
||
mkaply: can you help me understand what the differences between list.json (from https://bugzilla.mozilla.org/show_bug.cgi?id=1300201) and browsersearch.json (see https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/action/generate_browsersearch.py) are? The former is clearly for nsSearchService and the latter is for Android resources. But there seems to be a decent amount of duplication, and I'd like to see some unification here. What's the desired end state?
Comment 4•6 years ago
|
||
Sorry, BZ interface to :ni sucks -- mkaply, above, please.
Flags: needinfo?(mozilla)
Assignee | ||
Comment 5•6 years ago
|
||
I think at one point I tried to figure out the browsersearch.json stuff and how it all worked and ended up punting. The proper end state here would be one file (list.json) that would be the source of truth for all of our products (even Focus). It would be used as the basis for generating everything.
Flags: needinfo?(mozilla)
Comment 6•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #5) > I think at one point I tried to figure out the browsersearch.json stuff and > how it all worked and ended up punting. It's rather complicated only because it needs to be localized. In essence it walks some per-locale lists with a fallthrough mechanism to use defaults in the case that locales don't specify things, collecting tidbits of data. My guess is that locales are not really using the per-locale features now, although we could determine that fairly easily. > The proper end state here would be one file (list.json) that would be the > source of truth for all of our products (even Focus). > > It would be used as the basis for generating everything. OK, that's helpful. Is list.json constant across locales? (I think this is a goal of the search team, although I don't have a ticket tracking this. Link appreciated.) One wrinkle of browsersearch.json is that the some of the entries identify Android resources that are *very* Fennec specific -- they are literally names of files in the Fennec source tree. Presumably that doesn't play well with a global "basis for generating everything", and will need some consideration to unify. The build system could do things here if needed, like extract base64 encoded image data or SVG data from the JSON and produce the required files for the Android APK. Although I would prefer to significantly simplify the Fennec feature rather than build warts into list.json that do not have a future.
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #6) > (In reply to Mike Kaply [:mkaply] from comment #5) > > The proper end state here would be one file (list.json) that would be the > > source of truth for all of our products (even Focus). > > > > It would be used as the basis for generating everything. > > OK, that's helpful. Is list.json constant across locales? (I think this is > a goal of the search team, although I don't have a ticket tracking this. > Link appreciated.) Yes. There are two list.json files for browser and mobile that contain the information for each locale. They don't contain searchDefault and searchOrder yet, but that's in the works: https://searchfox.org/mozilla-central/source/browser/locales/search/list.json https://searchfox.org/mozilla-central/source/mobile/locales/search/list.json The list.json files that are a part of the build are generated based on those files, but there's no reason we couldn't just use that file as is. > One wrinkle of browsersearch.json is that the some of the entries identify > Android resources that are *very* Fennec specific -- they are literally > names of files in the Fennec source tree. Presumably that doesn't play well > with a global "basis for generating everything", and will need some > consideration to unify. I just took a quick glance at browsersearch.json and saw this: {"default": "Google", "regions": {"US": {"default": "Google", "engines": ["Google", "Yahoo", "Bing"]}}, "engines": ["Google", "Yahoo", "Bing"]} Is there sometimes something different? Note this is using names instead of engines. > > The build system could do things here if needed, like extract base64 encoded > image data or SVG data from the JSON and produce the required files for the > Android APK. Although I would prefer to significantly simplify the Fennec > feature rather than build warts into list.json that do not have a future. So it seems to be browsersearch.json is really only used to make SearchEngineManager.java work? Thats another thing I've never completely understood. SearchEngineManager.java versus the nsSearchService.js being used by Gecko. Are we not using the opensearch files in Fennec in some cases?
Updated•6 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 9•6 years ago
|
||
So here's the basic patch. Some things we need to think about. 1. What happens when you switch languages dynamically? Do we swap out all the default engine and leave user added engines alone? Do we have any way to do that? 2. What if your default engine is not in the new language? (Note we probably have this problem today, it just happens on restart)
Comment 10•6 years ago
|
||
> 1. What happens when you switch languages dynamically? Do we swap out all the default engine and leave user added engines alone? Do we have any way to do that?
That's a great question for product. Should we? Is search engine selection based on language really, or region or a combination of those two (locale)?
What if I use French Firefox in Germany? Should I get French or German search engines? :)
Comment 11•6 years ago
|
||
Also, I think you can remove `REQ_LOCALES_CHANGED_TOPIC` - there's no reason to rescan when user changes their requested locales if we can't update the app-locales in result. And if we do, we'll fire the app-locales-changed event.
Assignee | ||
Comment 12•6 years ago
|
||
To answer my own questions. When you switch locales, on restart, you get all your default engines replaced with the ones from the new locale. All custom added engines stay behind. You default is maintained unless it's not there anymore and then it gets switched to the default for that language. So it looks like we work the way we should, we just need to make it dynamic.
Assignee | ||
Comment 13•6 years ago
|
||
> Also, I think you can remove `REQ_LOCALES_CHANGED_TOPIC` - there's no reason to rescan when user changes their requested locales if we can't update the app-locales in result. And if we do, we'll fire the app-locales-changed event.
This was only done for Android it looks like. So if a user changed their locale on their phone, we updated properly.
Is that not still the case?
Assignee | ||
Comment 14•6 years ago
|
||
So intl:app-locales-changed is working perfectly. The awesome bar engines don't update when it is changed, but if I open a new window, I see the changes. So we're going to need a separate bug to have the one off search engines update when the locale changes.
Comment 15•6 years ago
|
||
> Is that not still the case? We should broadcast `intl:app-locales-changed` on Android as well *if* the change of the OS locales impacted Firefox locale selection. :) > So intl:app-locales-changed is working perfectly. Yay for unified locale management :)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•6 years ago
|
||
This new patch makes the one off searches change dynamically. It's pretty cool. I'm going to wait until Florian comes back to get this reviewed. As far as Android goes, I'm leaving that code alone for now until I can test it more.
Assignee | ||
Comment 18•6 years ago
|
||
Note I still need to do the android version of the build patch.
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8980632 [details] Bug 1437942 - Bundle all search engines with Firefox. https://reviewboard.mozilla.org/r/246786/#review253086 This would save me pushing the try builds for doing this l10n+search stuff better at https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b146fc37fde75bfa36167afbcbe4aecdf4ba355 across the line, so I'd love to see it get priority. ::: browser/locales/Makefile.in:64 (Diff revision 1) > > include $(topsrcdir)/config/rules.mk > > include $(topsrcdir)/toolkit/locales/l10n.mk > > -$(list-json): $(call mkdir_deps,$(SEARCHPLUGINS_PATH)) $(if $(IS_LANGUAGE_REPACK),FORCE) > +searchplugins:: $(srcdir)/search/list.json You can get rid of quite a bit more here -- basically, everything searchplugin-y.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 29•6 years ago
|
||
Try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a9dcf45174289e641a10706e089547c056e4a22 looks pretty healthy, but we'll need to update the "all files referenced" test at https://searchfox.org/mozilla-central/source/browser/base/content/test/static/browser_all_files_referenced.js#38.
Updated•6 years ago
|
Attachment #8980809 -
Flags: review?(core-build-config-reviews) → review?(ted)
Attachment #8980808 -
Flags: review?(core-build-config-reviews) → review?(ted)
Attachment #8980817 -
Flags: review?(core-build-config-reviews) → review?(ted)
Assignee | ||
Comment 30•6 years ago
|
||
mozreview-review |
Comment on attachment 8980817 [details] Bug 1437942 - Part 3b: Make mobile/android resource://search-plugins reference srcdir. https://reviewboard.mozilla.org/r/246998/#review253762 ::: mobile/android/components/search/moz.build:10 (Diff revision 1) > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +JAR_MANIFESTS += ['jar.mn'] > + > +with Files('**'): > + BUG_COMPONENT = ('Firefox', 'Awesomescreen') Is this BUG_COMPONENT right?
Assignee | ||
Comment 31•6 years ago
|
||
mozreview-review |
Comment on attachment 8980816 [details] Bug 1437942 - Part 3a: Move mobile/android searchplugins and search/list.json to mobile/android/components/search/searchplugins. https://reviewboard.mozilla.org/r/246996/#review253764
Attachment #8980816 -
Flags: review?(mozilla) → review+
Assignee | ||
Comment 32•6 years ago
|
||
mozreview-review |
Comment on attachment 8980807 [details] Bug 1437942 - Part 2a: Move browser/ searchplugins and search/list.json to browser/components/search/searchplugins. https://reviewboard.mozilla.org/r/246976/#review253766
Attachment #8980807 -
Flags: review?(mozilla) → review+
Assignee | ||
Comment 33•6 years ago
|
||
mozreview-review |
Comment on attachment 8980808 [details] Bug 1437942 - Part 2b: Make browser/ resource://search-plugins reference srcdir. https://reviewboard.mozilla.org/r/246978/#review253770 ::: browser/components/search/jar.mn:10 (Diff revision 2) > browser.jar: > content/browser/search/search.xml (content/search.xml) > content/browser/search/searchReset.xhtml (content/searchReset.xhtml) > content/browser/search/searchReset.js (content/searchReset.js) > + > + searchplugins/ (searchplugins/**) What do two stars mean in this context?
Attachment #8980808 -
Flags: review?(mozilla) → review+
Assignee | ||
Updated•6 years ago
|
Attachment #8980632 -
Attachment is obsolete: true
Comment 34•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #33) > Comment on attachment 8980808 [details] > Bug 1437942 - Part 2b: Make browser/ resource://search-plugins reference > srcdir. > > https://reviewboard.mozilla.org/r/246978/#review253770 > > ::: browser/components/search/jar.mn:10 > (Diff revision 2) > > browser.jar: > > content/browser/search/search.xml (content/search.xml) > > content/browser/search/searchReset.xhtml (content/searchReset.xhtml) > > content/browser/search/searchReset.js (content/searchReset.js) > > + > > + searchplugins/ (searchplugins/**) > > What do two stars mean in this context? It's a deep recursion through paths. So searchplugins/*, searchplugins/foo/*, searchplugins/foo/bar/*, etc.
Comment hidden (mozreview-request) |
Comment 36•6 years ago
|
||
mozreview-review |
Comment on attachment 8982277 [details] Bug 1437942 - Remove search engines from langpacks. https://reviewboard.mozilla.org/r/248220/#review254472 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/components/search/nsSearchService.js:3414 (Diff revision 1) > return; > } > > + let searchSettings; > + let locale = Services.locale.getAppLocaleAsBCP47(); > + Components.utils.reportError("*****************" + locale); Error: Use cu rather than components.utils [eslint: mozilla/use-cc-etc]
Comment hidden (mozreview-request) |
Comment 38•6 years ago
|
||
mozreview-review |
Comment on attachment 8982277 [details] Bug 1437942 - Remove search engines from langpacks. https://reviewboard.mozilla.org/r/248220/#review254564 ::: toolkit/components/search/nsSearchService.js:3419 (Diff revision 2) > + if ("locales" in json && > + locale in json.locales) { > + searchSettings = json.locales[locale]; > + } else { > + // json.default should always be there. > + // Should we assert? What does this first comment mean? "default" should have been in json, so it's weird that we ended up in the else-branch; or, it's OK we're in the else-branch because "default" will (or should) be in json? If the json doesn't look like what you expect, I would think we should LOG() and return like we do in the code right above this.
Attachment #8982277 -
Flags: review?(adw) → review+
Comment 39•6 years ago
|
||
mozreview-review |
Comment on attachment 8980808 [details] Bug 1437942 - Part 2b: Make browser/ resource://search-plugins reference srcdir. https://reviewboard.mozilla.org/r/246978/#review255194 Nothing makes me happier than patches that remove a bunch of Makefile rules! ::: browser/locales/moz.build (Diff revision 2) > > -with Files("search/**"): > - BUG_COMPONENT = ("Firefox", "Search") > - > -with Files("searchplugins/**"): > - BUG_COMPONENT = ("Firefox", "Search") Do these files wind up under some other bug component if you remove this?
Attachment #8980808 -
Flags: review?(ted) → review+
Comment 40•6 years ago
|
||
mozreview-review |
Comment on attachment 8980809 [details] Bug 1437942 - Pre: Drop unused variable. https://reviewboard.mozilla.org/r/246980/#review255196
Attachment #8980809 -
Flags: review?(ted) → review+
Comment 41•6 years ago
|
||
mozreview-review |
Comment on attachment 8980817 [details] Bug 1437942 - Part 3b: Make mobile/android resource://search-plugins reference srcdir. https://reviewboard.mozilla.org/r/246998/#review255198 This patch is even better than the other one!
Attachment #8980817 -
Flags: review?(ted) → review+
Comment 43•6 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #39) > Comment on attachment 8980808 [details] > Bug 1437942 - Part 2b: Make browser/ resource://search-plugins reference > srcdir. > > https://reviewboard.mozilla.org/r/246978/#review255194 > > Nothing makes me happier than patches that remove a bunch of Makefile rules! > > ::: browser/locales/moz.build > (Diff revision 2) > > > > -with Files("search/**"): > > - BUG_COMPONENT = ("Firefox", "Search") > > - > > -with Files("searchplugins/**"): > > - BUG_COMPONENT = ("Firefox", "Search") > > Do these files wind up under some other bug component if you remove this? Yes -- browser/components/search is already listed under "Firefox", "Search".
Updated•6 years ago
|
Blocks: nomakefiles
Comment 44•6 years ago
|
||
mkaply has a few issues to address in his patch, and then will squash these all down to land as a single atomic commit (with r=adw,ted.mielczarek). We can't bisect through this series so this is necessary.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 46•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=54b52472b1653b354d7dd5e603deb56824b9f971
Comment hidden (mozreview-request) |
Comment 48•6 years ago
|
||
mozreview-review |
Comment on attachment 8982277 [details] Bug 1437942 - Remove search engines from langpacks. https://reviewboard.mozilla.org/r/248220/#review255272 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/components/search/nsSearchService.js:3421 (Diff revision 3) > + searchSettings = json.locales[locale]; > + } else { > + // No locales were found, so use the JSON as is. > + // It should have a default section. > + if (!("default" in json)) { > + LOG("missing default in list.json: " + e); Error: 'e' is not defined. [eslint: no-undef]
Comment hidden (mozreview-request) |
Comment 50•6 years ago
|
||
mozreview-review |
Comment on attachment 8982277 [details] Bug 1437942 - Remove search engines from langpacks. https://reviewboard.mozilla.org/r/248220/#review255408 ::: browser/modules/ContentSearch.jsm:199 (Diff revision 5) > }); > this._processEventQueue(); > break; > + case "browser-search-service": > + if (data == "init-complete") { > + this._eventQueue.push({ nit: You can avoid duplicating this by moving the browser-search-service case before browser-search-engine-modified, and using break only when data != "init-complete". ::: toolkit/components/search/nsSearchService.js:3421 (Diff revision 5) > + searchSettings = json.locales[locale]; > + } else { > + // No locales were found, so use the JSON as is. > + // It should have a default section. > + if (!("default" in json)) { > + LOG("missing default in list.json"); When do we expect this to happen? ::: toolkit/components/search/nsSearchService.js:3513 (Diff revision 5) > if (searchRegion && searchRegion in searchSettings && > "searchDefault" in searchSettings[searchRegion]) { > this._searchDefault = searchSettings[searchRegion].searchDefault; > - } else { > + } else if ("searchDefault" in searchSettings.default) { > this._searchDefault = searchSettings.default.searchDefault; > + } else if ("searchDefault" in json.default) { Why is this "if ("searchDefault" in json.default)" test needed? In which case would this be false? If there are realistic chances of it happening, should we fail loudly in that case where we end up without any search default?
Assignee | ||
Comment 51•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8982277 [details] Bug 1437942 - Remove search engines from langpacks. https://reviewboard.mozilla.org/r/248220/#review255408 > nit: You can avoid duplicating this by moving the browser-search-service case before browser-search-engine-modified, and using break only when data != "init-complete". I tried to do that, but I thought the eslinter didn't like it. I'll check again. > When do we expect this to happen? This should really never happen. What is the right thing to do in our Javascript code in situations like this? This is really an "assert" > Why is this "if ("searchDefault" in json.default)" test needed? In which case would this be false? If there are realistic chances of it happening, should we fail loudly in that case where we end up without any search default? It really can never happen so we should fail loudly.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 55•6 years ago
|
||
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/f4a2094a00c5 Remove search engines from langpacks. r=adw.mielczarek
Comment 56•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f4a2094a00c5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Comment hidden (off-topic) |
Comment hidden (off-topic) |
Updated•6 years ago
|
tracking-firefox62:
--- → +
Comment 59•6 years ago
|
||
Backed out as requested by pascalc. Backout link: https://hg.mozilla.org/mozilla-central/rev/78c1a9ad32c674dc272490993c7ec5a259ac84f5
Comment 60•6 years ago
|
||
This was backed out due to l10n bustages, e.g. bug 1467040.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 61•6 years ago
|
||
I did some initial investigation into this. It looks like the *.xml files aren't being included in the localised builds. My suspicion would be this line in jar.mn (and the android jar.mn): https://hg.mozilla.org/mozilla-central/rev/f4a2094a00c5#l4.13 searchplugins/ (searchplugins/**) From looking on searchfox, we only seem to use a double star when doing something like (%browser/**/*.ftl), in all other cases of including directory contents we use a single star. I'm not 100% sure if a single star would include the images/ directory though (I hope it would), maybe look through some of the existing usages or ask a build config person.
Comment 62•6 years ago
|
||
Side note: we have localized builds on try now (for 5 locales).
Comment 63•6 years ago
|
||
Pretty sure that the hard-coded "let's repack searchplugins" in https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/l10n-repack.py#18 needs to go to fix this. Mark, thanks for the investigation, knowing what's missing made it easier to find why.
Comment 64•6 years ago
|
||
FYI, the respun builds work correctly and missing search engines was not the only bug reported, the "open in a new tab" context menu item was no longer working for localized builds and right-clicking on a link was proposing weird context menu items such as "add to dictionary". The awesome bar had also lost all of its history. All these bugs disappeared when the patch was backed out, mentioning in case it matters for the next landing of the patch.
Comment 65•6 years ago
|
||
(In reply to Pascal Chevrel:pascalc from comment #64) > FYI, the respun builds work correctly and missing search engines was not the > only bug reported, the "open in a new tab" context menu item was no longer > working for localized builds and right-clicking on a link was proposing > weird context menu items such as "add to dictionary". The awesome bar had > also lost all of its history. All these bugs disappeared when the patch was > backed out, mentioning in case it matters for the next landing of the patch. That happens when we break search (e.g. for Italian in ESR 52), and that's a bit surprising. I think the browser should still be functional, even if search plugins are missing for some reasons.
Assignee | ||
Comment 66•6 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #63) > Pretty sure that the hard-coded "let's repack searchplugins" in > https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/ > l10n-repack.py#18 needs to go to fix this. > > Mark, thanks for the investigation, knowing what's missing made it easier to > find why. That first searchplugins reference there is from when we used to have a top level searchplugins directory (and should probably be removed). I would think the chrome/**/searchplugins/*.xml line would grab the rest of the search engines. What's odd is that we are grabbing the list.json and the images directory, so it's almost like we're excluding the files.
Comment 67•6 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #61) > I did some initial investigation into this. It looks like the *.xml files > aren't being included in the localised builds. > > My suspicion would be this line in jar.mn (and the android jar.mn): > > https://hg.mozilla.org/mozilla-central/rev/f4a2094a00c5#l4.13 > > searchplugins/ > (searchplugins/**) > > From looking on searchfox, we only seem to use a double star when doing > something like (%browser/**/*.ftl), in all other cases of including > directory contents we use a single star. > > I'm not 100% sure if a single star would include the images/ directory > though (I hope it would), maybe look through some of the existing usages or > ask a build config person. I'm pretty confident that ** is deep pattern matching, so that **/* is equivalent, but I can definitely verify. (In my local builds, I'm quite confident it is.) I think it's much more likely that there's so l10n-repacking stuff that has to be removed. mkaply and I will iterate on it.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8982277 -
Flags: review?(ted) → review+
Updated•6 years ago
|
status-firefox60:
affected → ---
Comment hidden (mozreview-request) |
Comment 70•6 years ago
|
||
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/67f15f445506 Remove search engines from langpacks. r=adw
Comment 71•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/67f15f445506
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Comment 72•6 years ago
|
||
Comment on attachment 8980817 [details] Bug 1437942 - Part 3b: Make mobile/android resource://search-plugins reference srcdir. Dropping obsolete review request.
Attachment #8980817 -
Flags: review?(mozilla)
You need to log in
before you can comment on or make changes to this bug.
Description
•