Bug 3862

Summary:

Bidi control characters added by the fix to bug 3545 are actually rendered

Product:

WebKit

Reporter:

mitz

Component:

Layout and Rendering

Assignee:

Dave Hyatt <hyatt>

Status:

CLOSED FIXED

Severity:

Normal

Priority:

Version:

412

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Testcase	none
actual rendering	none
expected rendering	none
Proposed fix	hyatt: review+

mitz

Reported 2005-07-05 06:08:35 PDT

When using a font that contains glyphs for bidi control characters LRO and PDF, each run of visually- ordered Hebrew is enclosed by those glyphs. To reproduce: You need a font that contains those glyphs. For the testcase, the version of Arial that comes with Windows XP will do (disable all other versions of Arial). Open the testcase. Expected: Not to see control-character glyphs on either side of the Hebrew text (see screenshot). Actual: The Hebrew letters are enclosed by the LRO and PDF glyphs. Analysis: the fix for bug 3545 encloses each run of visually-ordered Hebrew in LRO-PDF, and adjusts the from and to members to include the LRO and PDF. It shouldn't. ATSUI will respect the LRO even if it occurs before the first character to be rendered. The patch corrects this by making the from-to range include only the original characters in the run, leaving the control characters outside.

Attachments
Testcase (362 bytes, text/html) 2005-07-05 06:09 PDT, mitz	no flags	Details
actual rendering (2.15 KB, image/png) 2005-07-05 06:10 PDT, mitz	no flags	Details
expected rendering (1.86 KB, image/png) 2005-07-05 06:11 PDT, mitz	no flags	Details
Proposed fix (1.22 KB, patch) 2005-07-05 06:12 PDT, mitz	hyatt: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

mitz

Comment 1 2005-07-05 06:09:24 PDT

Created attachment 2813 [details] Testcase

mitz

Comment 2 2005-07-05 06:10:40 PDT

Created attachment 2814 [details] actual rendering

mitz

Comment 3 2005-07-05 06:11:47 PDT

Created attachment 2815 [details] expected rendering

mitz

Comment 4 2005-07-05 06:12:40 PDT

Created attachment 2816 [details] Proposed fix

Joost de Valk (AlthA)

Comment 5 2005-07-05 13:01:58 PDT

Confirmed, weird stuff, hebrew is nice tho :P

Dave Hyatt

Comment 6 2005-07-09 14:08:56 PDT

Comment on attachment 2816 [details] Proposed fix r=me

Darin Adler

Comment 7 2005-07-10 06:50:10 PDT

Comment on attachment 2816 [details] Proposed fix Looks great to me too, although I don't see the need for the cast to (int) in the expression: (int)run->length - 1 since subtracting 1 from the unsigned value is going to make it into a signed value anyway.

mitz

Comment 8 2005-07-10 07:47:18 PDT

(In reply to comment #7) > (From update of attachment 2816 [details] [edit]) > Looks great to me too, although I don't see the need for the cast to (int) in > the expression: > > (int)run->length - 1 > > since subtracting 1 from the unsigned value is going to make it into a signed > value anyway. > Tell that to the compiler. Without the case, it says "warning: signed and unsigned type in conditional expression".

Justin Garcia

Comment 9 2005-07-23 14:02:33 PDT

Landing a fix and adding a manual test, since our layout test font doesn't include glyphs for LRO and PDF.

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