Bug 3838

Summary: Text runs unnecessarily split at EN L boundaries
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: VERIFIED FIXED    
Severity: Normal    
Priority: P2    
Version: 412   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on: 3733    
Bug Blocks:    
Attachments:
Description Flags
Testcase
none
Proposed patch
hyatt: review+
float_on_text_elements before
none
float_on_text_elements after none

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.