1280757 - Load hyphenation dictionaries from writable location

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[k.krish]

We are excluding the hyphenation dictionaries from the APK in the Bug 1095719. So when we exclude the hyphenation dictionary from the build, some reftests will fail. We need to package them separately and install them along with the build in this Bug 1276560.

We are loading the hyphenation dictionary into the test profile - sdcard/tests/reftest/profile/hyphenation/<files>

But we need to load the hyphenation dictionaries from the test profile directory.

Comment 1

[k.krish]

Attachment: Bug 1280757 - Load hyphenation from test profile directory

Review commit: https://reviewboard.mozilla.org/r/59558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59558/

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8763334 [details]
Bug 1280757 - Load hyphenation from test profile directory

Simon, Masatoshi: You are the owners or peers of intl/ - Can anyone of you review the patch or flag someone who can? :)

We'd like to add a sub directory of the profile directory as additional directory to look for hyphenation dictionaries.

Comment 3

[Masatoshi Kimura [:emk]]

Do you know why hyphenation dictionaries have been restricted to non-writable location? Or do you know who would have known that?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Masatoshi Kimura [:emk] from comment #3)
> Do you know why hyphenation dictionaries have been restricted to
> non-writable location? Or do you know who would have known that?

I know nothing about the history of the hyphenation manager unfortunately. However it is loading dictionaries from the process dir:
https://dxr.mozilla.org/mozilla-central/source/intl/hyphenation/glue/nsHyphenationManager.cpp#161-170

.. and this directory is writeable and not restricted - at least on Android. And that's exactly what we want to use on Android: To reduce the app size we do not want to ship the dictionaries with the app anymore. Instead we download them at runtime into the process dir. However we can't do this when running reftests, so we would like to push them to the test profile. That's how we handle other files (e.g.) that need to exist for testing too.

Comment 5

[Masatoshi Kimura [:emk]]

(In reply to Sebastian Kaspari (:sebastian) from comment #4)
> .. and this directory is writeable and not restricted - at least on Android.

But it is usually not writable on desktop platforms, although it is possible to install Firefox to writable location.

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 8763334 [details]
Bug 1280757 - Load hyphenation from test profile directory

https://reviewboard.mozilla.org/r/59558/#review57062

OK, let's try and see whats going on.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8763334 [details]
Bug 1280757 - Load hyphenation from test profile directory

Thank you!

Comment 8

[k.krish]

Comment on attachment 8763334 [details]
Bug 1280757 - Load hyphenation from test profile directory

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59558/diff/1-2/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8763334 [details]
Bug 1280757 - Load hyphenation from test profile directory

This patch already had r+ from Masatoshi and it doesn't seem that the patch has changed (Is this just a rebase?). You can go ahead and land it if there are no other issues.

Comment 10

[k.krish]

Yes it is just a rebase. I triggered a new try run for this too. I will land it asap

Comment 11

[k.krish]

Try Results :

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2f87cb99037&selectedJob=23121889

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/dc96e5ff02fd
Load hyphenation from test profile directory. r=sebastian

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dc96e5ff02fd