Closed Bug 1300198 Opened 8 years ago Closed 4 years ago

Move list.txt over to JSON once bug 1276739 is in and centralise

Categories

(SeaMonkey :: Search, defect)

defect
Not set
normal

Tracking

(seamonkey2.49esr wontfix, seamonkey2.53+ fixed, seamonkey2.57esr? affected)

RESOLVED FIXED
seamonkey 2.75
Tracking Status
seamonkey2.49esr --- wontfix
seamonkey2.53 + fixed
seamonkey2.57esr ? affected

People

(Reporter: mkaply, Assigned: iannbugzilla)

References

(Blocks 2 open bugs)

Details

(Whiteboard: SM2.53.3)

Attachments

(5 files, 8 obsolete files)

50.22 KB, image/png
Details
9.29 KB, patch
frg
: review+
Details | Diff | Splinter Review
208.76 KB, patch
frg
: review+
Details | Diff | Splinter Review
27.62 KB, patch
Details | Diff | Splinter Review
8.90 KB, patch
frg
: review+
Details | Diff | Splinter Review
In bug 1276739, we are moving to JSON files for search, replacing list.txt.

For browser, we will have one JSON file that is parsed at build time to create locale specific JSON files.

Once we get the code checked in, we'd like the change made in SeaMonkey so we can remove the list.txt code from search service.
Blocks: 1300209
Attached file First pass (obsolete) —
I've done a first pass at this that should work, but I've not completely verified.

I'm planning to remove list.txt support within the next week or so, so you'll need to make sure this works in seamonkey.
Flags: needinfo?(ewong)
Note this work was already done for thunderbird and that's what I've based the makefile changes on.

The list.json comes from looking at the list.txt and region.properties files in the locale repos.
Thanks Mike, I will look if it builds when the list.txt is removed. SeaMonkey in comm-central is broken because of other changes and we just keep it building for now while we fix the esr version up for a release.
(In reply to Mike Kaply [:mkaply] from comment #1)
> Created attachment 8979322 [details]
> First pass
> 
> I've done a first pass at this that should work, but I've not completely
> verified.
> 
> I'm planning to remove list.txt support within the next week or so, so
> you'll need to make sure this works in seamonkey.

So far, looks good.  Need to change the available locales support; but we can deal with
that later.
Flags: needinfo?(ewong)
Comment on attachment 8979322 [details]
First pass

As discussed in yesterdays status meeting.
Attachment #8979322 - Flags: feedback?(iann_bugzilla)

Unbitrotted the patch and updated list-json for the current shipped locales

Assignee: nobody → iann_bugzilla
Attachment #8979322 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8979322 - Flags: feedback?(iann_bugzilla)
Attachment #9059254 - Flags: review?(frgrahl)
Attachment #9059254 - Flags: approval-comm-esr60?
Attached patch Add jar.mn change (obsolete) — Splinter Review

There was a change to suite/locale/jar.mn missing

Attachment #9059254 - Attachment is obsolete: true
Attachment #9059254 - Flags: review?(frgrahl)
Attachment #9059254 - Flags: approval-comm-esr60?
Attachment #9059257 - Flags: review?(frgrahl)
Attachment #9059257 - Flags: approval-comm-esr60?

I've tested this against 2.57 en-US but worth feedback from other locales.

Attachment #9059257 - Attachment is obsolete: true
Attachment #9059257 - Flags: review?(frgrahl)
Attachment #9059257 - Flags: approval-comm-esr60?
Attachment #9059843 - Flags: feedback?(stefanh)
Attachment #9059843 - Flags: feedback?(frgrahl)
Comment on attachment 9059843 [details] [diff] [review]
Centralise search plugins and switch to json

Hmm overall this looks ok but is for comm-central which is broken. Please check the first patch from Mike for the additonal logic. The current patch is only valid after Bug 1437942 which is in 62. Missing at least the searchplugin logic in makefile.in.
Attachment #9059843 - Flags: feedback?(frgrahl) → feedback-

(In reply to Frank-Rainer Grahl (:frg) from comment #9)

Comment on attachment 9059843 [details] [diff] [review]
Centralise search plugins and switch to json

Hmm overall this looks ok but is for comm-central which is broken. Please
check the first patch from Mike for the additonal logic. The current patch
is only valid after Bug 1437942 which is in 62. Missing at least the
searchplugin logic in makefile.in.

If it was staying not centralised and under suite/locales then, yes, we'd need some logic in Makefile.in but as the patch puts them centrally under suite/components/search then that is done in that area's jar.mn

I have currently morphed the bug to include centralisation under suite/components/search but if you would prefer only switch to json done here and centralisation done elsewhere I can do that.

Flags: needinfo?(frgrahl)

Also, as far as I can see, there are no additional m-esr60 requirements to make this work in 2.57 (though I have only tested against en-US).

Comment on attachment 9059843 [details] [diff] [review]
Centralise search plugins and switch to json

Oki will check then.
Flags: needinfo?(frgrahl)
Attachment #9059843 - Flags: feedback- → feedback?(frgrahl)
Comment on attachment 9059843 [details] [diff] [review]
Centralise search plugins and switch to json

The en-US build is fine with the patch.

An l10n de build will not search and shows zero search engines in the prefs.
Attachment #9059843 - Flags: feedback?(frgrahl) → feedback-

Might have a dependency on bug 1441016.

Comment on attachment 9059843 [details] [diff] [review]
Centralise search plugins and switch to json

Cancelling request since I think frg made it obsolete.
Attachment #9059843 - Flags: feedback?(stefanh)
See Also: → 1630282
Attached patch Move list.txt to JSON (obsolete) — Splinter Review

This is the first step, other steps require patches on mozilla repo too.

Attachment #9059843 - Attachment is obsolete: true
Attachment #9141405 - Flags: review?(frgrahl)
Attachment #9141405 - Flags: approval-comm-release?
Attachment #9141405 - Flags: approval-comm-esr60?

This patch now also includes the port of:

  • Bug 1309304 - Move search python files to a central location
Attachment #9141405 - Attachment is obsolete: true
Attachment #9141405 - Flags: review?(frgrahl)
Attachment #9141405 - Flags: approval-comm-release?
Attachment #9141405 - Flags: approval-comm-esr60?
Attachment #9142031 - Flags: review?(frgrahl)
Attachment #9142031 - Flags: approval-comm-release?
Attachment #9142031 - Flags: approval-comm-esr60?
Comment on attachment 9142031 [details] [diff] [review]
Move list.txt to JSON v1.1

LGTM
Attachment #9142031 - Flags: review?(frgrahl)
Attachment #9142031 - Flags: review+
Attachment #9142031 - Flags: approval-comm-release?
Attachment #9142031 - Flags: approval-comm-release+
Attachment #9142031 - Flags: approval-comm-esr60?
Attachment #9142031 - Flags: approval-comm-esr60+
Summary: Move list.txt over to JSON once bug 1276739 is in → Move list.txt over to JSON once bug 1276739 is in and centralise
Attached patch Centralise patch (obsolete) — Splinter Review

As far as I can tell I have picked up all the changes to search engines, this ports the following bugs to SeaMonkey:

  • Bug 1276740 - Centralize all search plugins into mozilla-central
  • Bug 1445582 - Error out when the searchplugins list is empty or a plugin is missing
  • Bug 1320712 - Make UTF-8 searches work on Amazon
  • Bug 1332741 - Use chrome URL for Wikipedia image
  • Bug 1337560 - Use chrome icons for Yahoo and Amazon
  • Bug 1399509 - Update search URL and icon for Chambers (en-GB)
  • Bug 1419117 - [cs] Update icons of Firefox desktop search modules and use https
  • Bug 1406164 - We're bringing eBay back
  • Bug 1531751 - Correct search parameter for mapy.cz searchplugin

I did look at Bug 1410095 but that got reversed later by Bug 1547291

Do we want to distinguish between the Yahoo search engines for the locales, most appear as just Yahoo whereas other engines have a locale in brackets or their domain name?
More work might be required for DuckDuckGo but that can be a separate patch / bug.
I manually tested most of the search engines, where a search engine did not do SSL or no longer worked, it was removed from the list or partially disabled (sapo.xml)
I have tested that each locale's installer is created without an error and tested a random set of locales with new profiles to make sure the search engines appeared.

Attachment #9142756 - Flags: review?(frgrahl)
Attachment #9142756 - Flags: approval-comm-release?
Attachment #9142756 - Flags: approval-comm-esr60?

Updated version with some fixes and missed formatting changes.

Attachment #9142756 - Attachment is obsolete: true
Attachment #9142756 - Flags: review?(frgrahl)
Attachment #9142756 - Flags: approval-comm-release?
Attachment #9142756 - Flags: approval-comm-esr60?
Attachment #9143172 - Flags: review?(frgrahl)
Attachment #9143172 - Flags: approval-comm-release?
Attachment #9143172 - Flags: approval-comm-esr60?
Comment on attachment 9143172 [details] [diff] [review]
Centralise patch v1.1

LGTM

2 questions asked over irc but good either case.
Attachment #9143172 - Flags: review?(frgrahl)
Attachment #9143172 - Flags: review+
Attachment #9143172 - Flags: approval-comm-release?
Attachment #9143172 - Flags: approval-comm-release+
Attachment #9143172 - Flags: approval-comm-esr60?
Attachment #9143172 - Flags: approval-comm-esr60+

Port the relevant parts of the following bugs:

It also needs rebased versions of the following bugs on mozilla repo:

  • Bug 1410736 - Remove remaining uses of general.useragent.locale
  • Bug 1352539 - Move default search engine to list.json
  • Bug 1460232 - Update search test to handle missing region
  • Bug 1460190 - Handle search regions with default but no enginelist
  • Bug 1461347 - Remove unused enginename from JS, but add to properties
  • Bug 1461345 - Move browser.search.order to list.json
Attachment #9143475 - Flags: review?(frgrahl)
Attachment #9143475 - Flags: approval-comm-release?
Attachment #9143475 - Flags: approval-comm-esr60?

Preliminary packaging fixes only.

Blocks: 1636756

Fixes issue with zh-CN searchDefault and searchOrder.

Attachment #9150612 - Flags: review?(frgrahl)
Attachment #9150612 - Flags: approval-comm-release?
Attachment #9150612 - Flags: approval-comm-esr60?
Attachment #9143475 - Attachment is obsolete: true
Attachment #9143475 - Flags: review?(frgrahl)
Attachment #9143475 - Flags: approval-comm-release?
Attachment #9143475 - Flags: approval-comm-esr60?

refreshed before uploading this time

Attachment #9150612 - Attachment is obsolete: true
Attachment #9150612 - Flags: review?(frgrahl)
Attachment #9150612 - Flags: approval-comm-release?
Attachment #9150612 - Flags: approval-comm-esr60?
Attachment #9150613 - Flags: review?(frgrahl)
Attachment #9150613 - Flags: approval-comm-release?
Attachment #9150613 - Flags: approval-comm-esr60?
Comment on attachment 9150613 [details] [diff] [review]
Move default engine and search order v1.1

LGTM
Attachment #9150613 - Flags: review?(frgrahl)
Attachment #9150613 - Flags: review+
Attachment #9150613 - Flags: approval-comm-release?
Attachment #9150613 - Flags: approval-comm-release+
Attachment #9150613 - Flags: approval-comm-esr60?
Attachment #9150613 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/ee142fe19958
Move list.txt over to JSON once bug 1276739 is in and port |Bug 1309304 - Move search python files to a central location| and |Bug 1328713 - add regionOverrides to search/list.json| to SeaMonkey. r=frg
https://hg.mozilla.org/comm-central/rev/cc85a33a585d
Move list.txt over to JSON once bug 1276739 is in and centralise - centralisation part. r=frg
https://hg.mozilla.org/comm-central/rev/67d1157ecdb0
Move list.txt over to JSON once bug 1276739 is in and centralise - defaultengine and search order patch. r=frg

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey 2.75
Version: unspecified → Trunk
Blocks: 1642306
Whiteboard: SM2.53.3

Target 2.53.3
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/9bea18736dfb19eba392e2d85d63546a99caeb97
Move list.txt over to JSON once bug 1276739 is in and port |Bug 1309304 - Move search python files to a central location| and |Bug 1328713 - add regionOverrides to search/list.json| to SeaMonkey. r=frg a=frg
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/986bd56557cf0738780e50928980d711159f9059
Move list.txt over to JSON once bug 1276739 is in and centralise - centralisation part. r=frg a=frg
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/de0a25eaa2985ccfee5759f49937531f8f35eaca
Move list.txt over to JSON once bug 1276739 is in and centralise - defaultengine and search order patch. r=frg a=frg

You need to log in before you can comment on or make changes to this bug.