Closed
Bug 455647
Opened 16 years ago
Closed 16 years ago
[Indic] Firefox displays garbage Indic characters on parts of some English webpages
Categories
(Core :: Graphics, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b2
People
(Reporter: panemade, Assigned: karlt)
Details
(Keywords: fixed1.9.1, testcase, verified1.9.0.6)
Attachments
(6 files)
175.98 KB,
image/png
|
Details | |
27.91 KB,
image/png
|
Details | |
95.76 KB,
image/png
|
Details | |
524 bytes,
text/html
|
Details | |
2.40 KB,
patch
|
pavlov
:
review+
dveditz
:
approval1.9.0.6+
|
Details | Diff | Splinter Review |
2.70 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008070206 Firefox/3.0.1 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008070206 Firefox/3.0.1 Localized builds are working fine. e.g. when firefox downloaded for Punjabi or Gujarati locales but when started English firefox in pa_IN or gu_IN locales, English web pages are showing characters in Non-English font. Reproducible: Always Steps to Reproduce: 1. download tarball 2. start firefox as "LANG=mr_IN.UTF-8 ./firefox" or "LANG=pa_IN.UTF-8 ./firefox" 3. open following English webpage links http://fedoraproject.org/static/firefox/ http://news.bbc.co.uk/2/hi/business/7620127.stm https://bugzilla.mozilla.org/show_bug.cgi?id=381967 Actual Results: English pages are showing characters from locale in which firefox started where no translations are exist for that locale. Expected Results: English pages should be rendering in English characters only.
Reporter | ||
Comment 1•16 years ago
|
||
Reporter | ||
Comment 2•16 years ago
|
||
Reporter | ||
Comment 3•16 years ago
|
||
Reporter | ||
Comment 4•16 years ago
|
||
All attached snapshots are taken with firefox binary English tarball firefox-3.0.1.tar.bz2 and started firefox on fedora linux as LANG=pa_IN.UTF-8 ./firefox
Updated•16 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 5•16 years ago
|
||
Still an issue on trunk.
Assignee: nobody → mozbugz
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•16 years ago
|
||
Simpler testcase. You wont need to use any particular locale to reproduce, but you will need lohit-fonts for this testcase. (I think different testcases could show the same problem with different fonts though.)
Assignee | ||
Comment 7•16 years ago
|
||
The problem is in creating TextRuns. First the fast path is attempted, and it calls AddGlyphRun with aStartCharIndex == 0. The fast path fails and falls back to the itemizing path. The itemizing path also calls AddGlyphRun with aStartCharIndex == 0 and a different font. The would be fine except that aForceNewRun == PR_TRUE is passed, which means that two different glyph runs with different fonts are recorded for the first run. The comparator used for the sort in SortGlyphRuns says the order of GlyphRuns with the same character offset is not important so the order becomes random and a random font gets used to render the glyphs (the indices of which were selected from a specific font).
Assignee | ||
Comment 8•16 years ago
|
||
The documentation for pango_itemize() says "Each byte of text will be contained in exactly one of the items in the returned list; the generated list of items will be in logical order (the start offsets of the items are ascending)." So there is no need to force a new run and sort the runs.
Attachment #339182 -
Flags: review?(roc)
Assignee | ||
Comment 9•16 years ago
|
||
For those with builds without a patch the workaround (as discovered by smontagu) is to set the preference "browser.display.auto_quality_min_font_size" to 0. (My initial hunch of a problem with lohit fonts was wrong.)
It does look like we shouldn't need to sort, but the code you're removing was deliberately added for bug 362682, so I think you should get review from Behdad and/or Stuart.
Assignee | ||
Comment 11•16 years ago
|
||
Comment on attachment 339182 [details] [diff] [review] replace old glyph run and add an assertion Stuart, we only ever increase utf16Offset so I can't think of any reason why we'd need to sort.
Attachment #339182 -
Flags: review?(roc) → review?(pavlov)
Comment 12•16 years ago
|
||
By the way, there are two bugs involved here: apart from the textrun bug that Karl found, the fact that we are trying to use the Punjabi font for English text in the first place is an example of bug 91190.
Assignee | ||
Comment 13•16 years ago
|
||
Assignee | ||
Comment 14•16 years ago
|
||
I think the effects of this bug are significant enough that it should be fixed for 1.9.1, and ported to 1.9.0.x after sufficient bake-time. The bug would be expected to affect users in non-Latin locales viewing pages with Latin text in a non-Latin-specific charset without any lang attribute and without any Content-Language header. Whether users see the bug depends on which Latin characters are supported in the font for their native language. Many fonts support most Latin characters, so the effects would only be seen either with fonts designed for a specific script/language or with less common (Latin-1 supplement) characters.
Flags: wanted1.9.0.x?
Flags: blocking1.9.1?
Comment 15•16 years ago
|
||
Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release) for 1.9.0.x. However, we'll look at a reviewed and baked patch.
Flags: wanted1.9.0.x? → wanted1.9.0.x-
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
Stuart, can we get a review here?
Updated•16 years ago
|
Attachment #339182 -
Flags: review?(pavlov) → review+
Assignee | ||
Comment 17•16 years ago
|
||
Fixed: http://hg.mozilla.org/mozilla-central/rev/8feeaada71eb In testsuite: http://hg.mozilla.org/mozilla-central/rev/c8b770c2fe89 http://hg.mozilla.org/mozilla-central/rev/f404bcc1b7f6
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Reporter | ||
Comment 18•16 years ago
|
||
is there any build available to test? Is this patch included in latest nightly builds 3.1b1 ?
Assignee | ||
Comment 19•16 years ago
|
||
Not 3.1b1, but in nightly builds: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
Reporter | ||
Comment 20•16 years ago
|
||
Thanks Karl for your help and patch here. I see that latest nightly builds fixed above reported problem.
Reporter | ||
Comment 21•16 years ago
|
||
Will be good if this patch will be available in 3.0.x branch also. Can we have this fix applied to 3.0.5 branch?
Comment 22•16 years ago
|
||
Yep, fix in 1.9.0.x would be great.
Assignee | ||
Updated•16 years ago
|
Attachment #339182 -
Flags: approval1.9.0.6?
Assignee | ||
Comment 23•16 years ago
|
||
Comment on attachment 339182 [details] [diff] [review] replace old glyph run and add an assertion Requesting approval for 1.9.0.6. This patch changes two lines of code in one function, plus adds an assertion. A reftest would land with it. See comment 14 for discussion on what this bug effects and attachment 339013 [details] for an example of the problem. The small number of changes makes risk very low.
Assignee | ||
Comment 24•16 years ago
|
||
Fedora bug (so I don't lose it): https://bugzilla.redhat.com/show_bug.cgi?id=460865
Reporter | ||
Comment 25•16 years ago
|
||
Thanks Karl, I have tested your patch on 1.9.0.x branch also. Patch is working fine and this bug looks like get fixed on 1.9.0.x branch also along with 1.9.1 branch.
Comment 26•16 years ago
|
||
I tested nightly build for 3.1 (1.9.1) and it is working for my language. But Will it be possible to backport to 3.0.0.x?
Comment 27•16 years ago
|
||
(In reply to comment #23) > Requesting approval for 1.9.0.6. This patch changes two lines of code in one > function, plus adds an assertion. A reftest would land with it. : > The small number of changes makes risk very low. Thank you Karl for your work on this. I just want to chime in in agreement that it would be very good to get this patch included on the stable branch - the consequences for Indic users are quite serious: a lot of English pages are quite unreadable.
Comment 28•16 years ago
|
||
Based on comments here, in irc and on .l10n.in, marking this VERIFIED for trunk. Sam, is having this not shown up any beta 2 blockers good enough as baking?
Status: RESOLVED → VERIFIED
Comment 29•16 years ago
|
||
Yes, this has baked on trunk long enough and we'll look at the approval request when we start doing 1.9.0.6 triage.
Comment 30•16 years ago
|
||
Comment on attachment 339182 [details] [diff] [review] replace old glyph run and add an assertion Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #339182 -
Flags: approval1.9.0.6? → approval1.9.0.6+
Updated•16 years ago
|
Keywords: fixed1.9.1
Assignee | ||
Comment 31•16 years ago
|
||
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1228973642&maxdate=1228973890&who=karlt%2B%25karlt.net
Keywords: fixed1.9.0.6
Comment 32•15 years ago
|
||
Verified fixed in 1.9.0.6 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.6pre) Gecko/2009010604 GranParadiso/3.0.6pre.
Keywords: fixed1.9.0.6 → verified1.9.0.6
You need to log in
before you can comment on or make changes to this bug.
Description
•