Last Comment Bug 685214 - Hyphenation dictionary will not load correctly if Firefox is installed to a unicode path on Windows
: Hyphenation dictionary will not load correctly if Firefox is installed to a u...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Jonathan Kew (:jfkthame)
:
Mentors:
Depends on: 655337
Blocks: 253317
  Show dependency treegraph
 
Reported: 2011-09-07 10:24 PDT by Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
Modified: 2011-09-15 07:35 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, use URI spec rather than utf8 file path to specify hyphenation dictionary (7.22 KB, patch)
2011-09-10 07:42 PDT, Jonathan Kew (:jfkthame)
benjamin: review+
Details | Diff | Splinter Review

Description Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-09-07 10:24:47 PDT
The current way we load hyphenation dictionaries is to call GetNativePath on a nsIFile. This method can fail on Windows, because on Windows not every file can be represented by a char*: the only reliable way to represent a file path is with a WCHAR*/PRUnichar*.

http://hg.mozilla.org/mozilla-central/annotate/787ebf02a8c1/intl/hyphenation/src/nsHyphenator.cpp#l50

This can probably be fixed by bug 65537, but if not we need to modify libhyphen to take a windows-style path on Windows.
Comment 1 :Ehsan Akhgari 2011-09-08 08:00:56 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #0)
> This can probably be fixed by bug 65537, but if not we need to modify
> libhyphen to take a windows-style path on Windows.

Correction: bug 655337.
Comment 2 Jonathan Kew (:jfkthame) 2011-09-10 07:42:21 PDT
Created attachment 559690 [details] [diff] [review]
patch, use URI spec rather than utf8 file path to specify hyphenation dictionary

As the code only uses very basic stdio calls to access the dictionary, it looks like we could easily #define those functions in the hnjalloc.h header (which is a custom mozilla version anyway) in order to work around the issue here without having to actually patch the hyphen.c source (which I'd prefer to avoid if possible).

Here's a first attempt at a simple implementation - this replaces fopen, fgets and fclose (for the libhyphen code only) so that the string we pass to hnj_hyphen_load is a URL spec instead of just a pathname, and nsIInputStream is used to read from it - basically as Ehsan suggested in bug 655337 comment #6, point 3.

This version avoids changing the API of hnj_hyphen_load (e.g. to take an nsIURI instead of a C string), so that we don't have to patch core libhyphen code.

This should also provide a basis for loading hyphenation dictionaries from resource:// URLs referencing JARs, rather than individual files; as such, it's a first step towards fixing bug 655337.
Comment 3 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-09-14 09:21:25 PDT
Comment on attachment 559690 [details] [diff] [review]
patch, use URI spec rather than utf8 file path to specify hyphenation dictionary

Looks good to me.

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