Bug 3838 - Text runs unnecessarily split at EN L boundaries
Summary: Text runs unnecessarily split at EN L boundaries
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on: 3733
Blocks:
  Show dependency treegraph
 
Reported: 2005-07-03 06:35 PDT by mitz
Modified: 2005-07-19 22:16 PDT (History)
0 users

See Also:


Attachments
Testcase (290 bytes, text/html)
2005-07-03 06:36 PDT, mitz
no flags Details
Proposed patch (932 bytes, patch)
2005-07-06 06:20 PDT, mitz
hyatt: review+
Details | Formatted Diff | Diff
float_on_text_elements before (178.19 KB, image/png)
2005-07-06 14:14 PDT, mitz
no flags Details
float_on_text_elements after (178.26 KB, image/png)
2005-07-06 14:14 PDT, mitz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2005-07-03 06:35:47 PDT
[This is a followup on bug 3733]

bidiReorderLine unnecessarily splits text runs almost whenever there is an EN followed by an L (such as in 
"for 15px"). For example, the render tree of the testcase includes:
          text run at (0,0) width 236: "The render tree should contain only 1"
          text run at (236,0) width 313: "run for this line, despite the number in the middle."
where a single text run for the entire line could be used.

After the patch for bug 3733 is landed, an extra if statement in the case of L after EN should be 
introduced as already described in the comment in the patch, and all affected layout tests should be 
checked.
Comment 1 mitz 2005-07-03 06:36:39 PDT
Created attachment 2773 [details]
Testcase
Comment 2 Geoffrey Garen 2005-07-05 17:37:56 PDT
(In reply to comment #0)

OK. The 3733 patch is landed.
Comment 3 mitz 2005-07-06 06:20:44 PDT
Created attachment 2830 [details]
Proposed patch
Comment 4 mitz 2005-07-06 14:14:14 PDT
Created attachment 2836 [details]
float_on_text_elements before
Comment 5 mitz 2005-07-06 14:14:55 PDT
Created attachment 2837 [details]
float_on_text_elements after
Comment 6 mitz 2005-07-06 14:28:57 PDT
Comment on attachment 2830 [details]
Proposed patch

The patch changes the layout trees of 136 layout tests, by joining runs.

I have rendered each of the 136 into 800x800 PNG files, with and without the
patch, and then compared the results. Differences were found in only 2 out of
the 136 cases:
  css1/box_properties/clear_float.html
and
  css1/box_properties/float_on_text_elements.html.
The differences are minimal and are due to the ability to better distribute the
spacing when justifying a longer run. See the attachments for one comparison.
The other one is similar.
Comment 7 Dave Hyatt 2005-07-09 13:58:23 PDT
Comment on attachment 2830 [details]
Proposed patch

Excellent. r=me.