Note: There are a few cases of duplicates in user autocompletion which are being worked on.

include hyphenation dictionaries in omnijar

RESOLVED FIXED in mozilla10

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

Trunk
mozilla10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Now that bug 655337 has landed, we no longer need to provide the bundled hyphenation dictionaries as separate files on disk in order for Firefox to find and use them; they can be loaded directly from omnijar. This will reduce our installed footprint, which is a good thing.
(Assignee)

Comment 1

6 years ago
Created attachment 565472 [details] [diff] [review]
patch, include hyphenation files in omnijar

AFAICS, this is trivial - we can add the hyphenation directory to OMNIJAR_FILES and it should Just Work.
Assignee: nobody → jfkthame
Attachment #565472 - Flags: review?(benjamin)
You might want to update package-manifest.in and removed-files.in as well:

http://mxr.mozilla.org/comm-central/search?string=hyphenation&find=installer&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
Attachment #565472 - Flags: review?(benjamin) → review?(mh+mozilla)
Comment on attachment 565472 [details] [diff] [review]
patch, include hyphenation files in omnijar

Sorry I didn't come to this earlier.
Attachment #565472 - Flags: review?(mh+mozilla) → review+

Comment 4

6 years ago
To clarify, you absolutely want to also update removed-files.in. Place the entries in the MOZ_OMNIJAR section.
(Assignee)

Comment 5

6 years ago
Created attachment 569981 [details] [diff] [review]
v2, include hyphenation files in omnijar, and remove the separate files

(In reply to Michael Wu [:mwu] from comment #4)
> To clarify, you absolutely want to also update removed-files.in. Place the
> entries in the MOZ_OMNIJAR section.

So like this, yes? Also removed from package-manifest.in as per comment #2.
Attachment #569981 - Flags: review?(mwu)

Comment 6

6 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 569981 [details] [diff] [review] [diff] [details] [review]
> v2, include hyphenation files in omnijar, and remove the separate files

I think you also need to add a line for just "hyphenation/" to removed-files.in to get rid of the then empty directory.

Comment 7

6 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 569981 [details] [diff] [review] [diff] [details] [review]
> v2, include hyphenation files in omnijar, and remove the separate files
> 
> (In reply to Michael Wu [:mwu] from comment #4)
> > To clarify, you absolutely want to also update removed-files.in. Place the
> > entries in the MOZ_OMNIJAR section.
> 
> So like this, yes? Also removed from package-manifest.in as per comment #2.

package-manifest.in shouldn't be updated AFAIK. Without it in the package-manifest, the hyphenation files won't get copied to the staging directory, and the omnijar code only takes files out of the staging directory.

(In reply to Dave Garrett from comment #6)
> (In reply to Jonathan Kew (:jfkthame) from comment #5)
> > Created attachment 569981 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > v2, include hyphenation files in omnijar, and remove the separate files
> 
> I think you also need to add a line for just "hyphenation/" to
> removed-files.in to get rid of the then empty directory.

Yup.
(Assignee)

Comment 8

6 years ago
(In reply to Michael Wu [:mwu] from comment #7)
> package-manifest.in shouldn't be updated AFAIK. Without it in the
> package-manifest, the hyphenation files won't get copied to the staging
> directory, and the omnijar code only takes files out of the staging
> directory.

Right - I had just pushed it to tryserver, which confirmed this by failing the hyphenation tests. Hence dropping that from the patch.
(Assignee)

Comment 9

6 years ago
Created attachment 570044 [details] [diff] [review]
v2.1, include hyphenation files in omnijar, and remove the separate files

Restored the required package-manifest.in entries.
Attachment #569981 - Attachment is obsolete: true
Attachment #569981 - Flags: review?(mwu)
Attachment #570044 - Flags: review?(mwu)

Comment 10

6 years ago
Comment on attachment 570044 [details] [diff] [review]
v2.1, include hyphenation files in omnijar, and remove the separate files

Looks good.
Attachment #570044 - Flags: review?(mwu) → review+
(Assignee)

Comment 11

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/782f5b321370
Target Milestone: --- → mozilla10
(Assignee)

Updated

6 years ago
Attachment #565472 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/782f5b321370
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Blocks: 698451
Blocks: 701428
You need to log in before you can comment on or make changes to this bug.