Bug 3599

Summary:

Incorrect layout of explicit embedding levels

Product:

WebKit

Reporter:

mitz

Component:

Layout and Rendering

Assignee:

Dave Hyatt <hyatt>

Status:

CLOSED FIXED

Severity:

Normal

CC:

bugs.mano

Priority:

Version:

412

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
testcase	none
Proposed fix	none
Proposed fix	darin: review+
Testcase	none
Testcase	none

mitz

Reported 2005-06-17 15:47:08 PDT

Summary: The layout is wrong in many cases of explicit Unicode embedding (either by a <span> with direction property or by Unicode embedding characters) occurs. To Reproduce: See testcase. Expected/Actual: Testcase includes a description of expected rendering. Firefox renders as expected. Note that mirroring behavior is also affected by a separate ATSUI issue with certain fonts. Use Lucida Grande for testing. Analysis: The testcase actually expose three issues in bidi.cpp: 1. bidiReorderLine() not closing the "dangling" run when it sees an explicit embedding character (or PDF). 2. embed() not setting up the new run correctly following an embed/pop. 3. bidiReorderLine ()'s implementation of rule L2 of the Unicode bidi algorithm is broken for lines where the embedding levels aren't unimodal (the index variable is advancing too fast). The proposed patch addresses these issues and fixes all the lines in the testcase. Testcase can be used as a new layout test.

Attachments
testcase (1.02 KB, text/html) 2005-06-17 15:49 PDT, mitz	no flags	Details
Proposed fix (1.36 KB, patch) 2005-06-17 15:59 PDT, mitz	no flags	Details Formatted Diff Diff
Proposed fix (774 bytes, patch) 2005-06-22 14:51 PDT, mitz	darin: review+	Details Formatted Diff Diff
Testcase (1.12 KB, text/html) 2005-06-23 05:58 PDT, mitz	no flags	Details
Testcase (1.12 KB, text/html) 2005-06-24 12:36 PDT, mitz	no flags	Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

mitz

Comment 1 2005-06-17 15:49:17 PDT

Created attachment 2453 [details] testcase

mitz

Comment 2 2005-06-17 15:59:11 PDT

Created attachment 2454 [details] Proposed fix

mitz

Comment 3 2005-06-18 13:50:29 PDT

Comment on attachment 2454 [details] Proposed fix Pulling the patch since it causes Safari to hang under some hard-to-reproduce circumstances.

mitz

Comment 4 2005-06-21 05:41:27 PDT

I've split the 3rd issue (implementation of rule L2) off into bug 3633 .

mitz

Comment 5 2005-06-22 14:51:12 PDT

Created attachment 2551 [details] Proposed fix

Darin Adler

Comment 6 2005-06-22 19:45:29 PDT

If the test case requires using Lucida Grande, then we should add CSS to make it do that, instead of relying on the default font.

Darin Adler

Comment 7 2005-06-22 19:47:46 PDT

Comment on attachment 2551 [details] Proposed fix I'm not really qualified to say whether this code is better or not, since I don't understand the bidi algorithm nor the details of our implementation of it. But as long as this fixes this test case and doesn't break any existing cases, I say r=me.

mitz

Comment 8 2005-06-23 05:58:19 PDT

Created attachment 2582 [details] Testcase Specified the use of Lucida Grande and added an item.

mitz

Comment 9 2005-06-24 12:36:47 PDT

Created attachment 2632 [details] Testcase Modified testcase so that the textual description of the expected layout (digits in increasing orders) would be correct.

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