CLOSED FIXED 3599
Incorrect layout of explicit embedding levels 
https://bugs.webkit.org/show_bug.cgi?id=3599
Summary Incorrect layout of explicit embedding levels
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
DetailsFormatted DiffDiff
Proposed fix (774 bytes, patch)
2005-06-22 14:51 PDT, mitz 		darin: review+
DetailsFormatted DiffDiff
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.