[Indic] Firefox displays garbage Indic characters on parts of some English webpages

VERIFIED FIXED in mozilla1.9.1b2

Status

()

Core
Graphics
P3
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Parag Nemade, Assigned: karlt)

Tracking

({fixed1.9.1, testcase, verified1.9.0.6})

unspecified
mozilla1.9.1b2
x86
Linux
fixed1.9.1, testcase, verified1.9.0.6
Points:
---
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Reporter)

Description

9 years ago
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

9 years ago
Created attachment 339012 [details]
snapshot for webpage http://news.bbc.co.uk/2/hi/business/7620127.stm
(Reporter)

Comment 2

9 years ago
Created attachment 339013 [details]
output for http://fedoraproject.org/static/firefox/
(Reporter)

Comment 3

9 years ago
Created attachment 339014 [details]
while reporting bug to mozilla bugzilla I got this snapshot
(Reporter)

Comment 4

9 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
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 5

9 years ago
Still an issue on trunk.
Assignee: nobody → mozbugz
Status: NEW → ASSIGNED
(Assignee)

Comment 6

9 years ago
Created attachment 339173 [details]
simple testcase with Lohit Punjabi, DejaVu Sans

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)

Updated

9 years ago
Keywords: testcase
(Assignee)

Comment 7

9 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

9 years ago
Created attachment 339182 [details] [diff] [review]
replace old glyph run and add an assertion

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

9 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

9 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)
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

9 years ago
Created attachment 339197 [details] [diff] [review]
fallback reftest
(Assignee)

Comment 14

9 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?
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

9 years ago
Attachment #339182 - Flags: review?(pavlov) → review+
(Assignee)

Comment 17

9 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
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
(Reporter)

Comment 18

9 years ago
is there any build available to test? Is this patch included in latest nightly builds 3.1b1 ?
(Assignee)

Comment 19

9 years ago
Not 3.1b1, but in nightly builds:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
(Reporter)

Comment 20

9 years ago
Thanks Karl for your help and patch here. I see that latest nightly builds fixed above reported problem.
(Reporter)

Comment 21

9 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

9 years ago
Yep, fix in 1.9.0.x would be great.
(Assignee)

Updated

9 years ago
Attachment #339182 - Flags: approval1.9.0.6?
(Assignee)

Comment 23

9 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

9 years ago
Fedora bug (so I don't lose it):
https://bugzilla.redhat.com/show_bug.cgi?id=460865
(Reporter)

Comment 25

9 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

9 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

9 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.
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
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 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+
Keywords: fixed1.9.1
(Assignee)

Comment 31

9 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
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.