Closed
Bug 643199
Opened 14 years ago
Closed 14 years ago
Correctly remove outdated L10n files on update
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(blocking-seamonkey2.1 final+)
RESOLVED
FIXED
seamonkey2.1final
Tracking | Status | |
---|---|---|
blocking-seamonkey2.1 | --- | final+ |
People
(Reporter: kairo, Assigned: ewong)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
As noted in bug 633394, we leave around old L10n files on update, which was made very visible with the omnijar switch - the reason is that we don't generate a new removed-files with the correct locale code when doing the repacks.
The general code for this will be done on the toolkit side (bug 592574), we just need a smaller patch to "activate" it and make sure our removed-files gets the correct variables when regenerating it from locales/Makefile.
Reporter | ||
Comment 1•14 years ago
|
||
This patch does the SeaMonkey-specific part, but the l10n.mk part is of course needed for it (if that doesn't get accepted, we can do all on our side, but doing it there is better).
Attachment #520488 -
Flags: review?(bugspam.Callek)
Comment 2•14 years ago
|
||
Comment on attachment 520488 [details] [diff] [review]
v1: add needed defines
r+ conditional on if the m-c bug lands. [I _hope_ it is accepted into m-2.0 there]
Attachment #520488 -
Flags: review?(bugspam.Callek) → review+
Reporter | ||
Comment 3•14 years ago
|
||
Putting this on the radar for final. Looks like nothing is moving in m-c there, so we might need to go with the "dirty" solution of putting in all possible locale files explicitely.
blocking-seamonkey2.1: --- → final+
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #529927 -
Flags: feedback?(bugspam.Callek)
Reporter | ||
Updated•14 years ago
|
Assignee: kairo → ewong
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #529927 -
Attachment is obsolete: true
Attachment #529927 -
Flags: feedback?(bugspam.Callek)
Attachment #530482 -
Flags: review?(bugspam.Callek)
Comment 6•14 years ago
|
||
Comment on attachment 530482 [details] [diff] [review]
Correctly remove outdated L10n files on update.
>-chrome/@AB_CD@.manifest
>+chrome/ca.manifest
>+chrome/de.manifest
>+chrome/es-ES.manifest
need en-US here as well
> #ifdef MOZ_OMNIJAR
>- chrome/@AB_CD@.jar
>+ chrome/ca.jar
>+ chrome/de.jar
>+ chrome/es-ES.jar
Same here
>+ extensions/langpack-zh-CN@chatzilla.mozilla.org/chrome/venkman.jar
>+ extensions/langpack-ca@chatzilla.mozilla.org/chrome.manifest
I'm going to take this section as an example of what I mean.... I'd like ALL the chatzilla entries above all the venkman entries (like you do have here), BUT I want it also grouped by locale above file.
So you'd have
extensions/langpack-ca@chatzilla.mozilla.org/chrome/venkman.jar
extensions/langpack-ca@chatzilla.mozilla.org/chrome.manifest
....
extensions/langpack-zh-CN@chatzilla.mozilla.org/chrome/venkman.jar
extensions/langpack-zh-CN@chatzilla.mozilla.org/chrome.manifest
...
make sense?
Attachment #530482 -
Flags: review?(bugspam.Callek) → review-
Comment 7•14 years ago
|
||
(In reply to comment #6)
> >+ extensions/langpack-zh-CN@chatzilla.mozilla.org/chrome/venkman.jar
...wait huh, this was broken before. can you make this chatzilla.jar [instead of venkman.jar] while you're here please?
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #530482 -
Attachment is obsolete: true
Attachment #530509 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #530509 -
Attachment is obsolete: true
Attachment #530509 -
Flags: review?(bugspam.Callek)
Attachment #530517 -
Flags: review?(bugspam.Callek)
Comment 10•14 years ago
|
||
Comment on attachment 530517 [details] [diff] [review]
Correctly remove outdated L10n files on update. (v4)
>+ extensions/langpack-en-US@chatzilla.mozilla.org/chrome/chatzilla.jar
>+ extensions/langpack-en-US@chatzilla.mozilla.org/chrome.manifest
>+ extensions/langpack-en-US@chatzilla.mozilla.org/install.js
>+ extensions/langpack-en-US@chatzilla.mozilla.org/install.rdf
>+ extensions/langpack-en-US@chatzilla.mozilla.org.xpi
We don't need en-US for chatzilla here (its a langpack)
>+ extensions/langpack-en-US@venkman.mozilla.org/chrome/venkman.jar
>+ extensions/langpack-en-US@venkman.mozilla.org/chrome.manifest
>+ extensions/langpack-en-US@venkman.mozilla.org/install.rdf
>+ extensions/langpack-en-US@venkman.mozilla.org.xpi
Same here.
Also chatzilla and venkman have more locales listed than needed, but it is NOT NECESSARY to fix for this bug (we can do it in a followup) as them being listed is not a problem.
The chatzilla locale list: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/irc/locales/all-locales
The Venkman Locale List: http://mxr.mozilla.org/comm-central/source/mozilla/extensions/venkman/locales/all-locales
r+ with those en-US entries mentioned removed.
Attachment #530517 -
Flags: review?(bugspam.Callek) → review+
Assignee | ||
Comment 11•14 years ago
|
||
Attachment #530517 -
Attachment is obsolete: true
Attachment #530522 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 12•14 years ago
|
||
What's the situation here? Reading comment 2 suggests that "v1: add needed defines" cannot land since the Core bug didn't land. Still that attachment has not been obsoleted. Thus I don't know what actually needs to land, and whether all dependencies have been met.
Clearing checkin-needed request until the above is resolved.
Keywords: checkin-needed
Comment 13•14 years ago
|
||
Comment on attachment 520488 [details] [diff] [review]
v1: add needed defines
Callek wrote via PM: "Just ewongs patch" -> obsoleting first patch
Attachment #520488 -
Attachment is obsolete: true
Comment 14•14 years ago
|
||
Comment on attachment 530522 [details] [diff] [review]
Correctly remove outdated L10n files on update. (v5) [Checkin: comment 14]
http://hg.mozilla.org/comm-central/rev/87dfb7e86938
http://hg.mozilla.org/releases/comm-2.0/rev/35ac22905535
Attachment #530522 -
Attachment description: Correctly remove outdated L10n files on update. (v5) → Correctly remove outdated L10n files on update. (v5) [Checkin: comment 14]
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
Reporter | ||
Comment 15•14 years ago
|
||
Yes, thanks, we want ewongs patch as mine would depend on a Mozilla fix we didn't get yet, not in trunk and of course not in mozilla-2.0 - thanks for taking care!
Updated•14 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•