Bug 3838 – Text runs unnecessarily split at EN L boundaries

• Component: Layout and Rendering
• Status: VERIFIED FIXED

[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.

Created attachment 2773 [details]
Testcase

[Geoffrey Garen]

(In reply to comment #0)

OK. The 3733 patch is landed.

Created attachment 2830 [details]
Proposed patch

Created attachment 2836 [details]
float_on_text_elements before

Created attachment 2837 [details]
float_on_text_elements after

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.

[Dave Hyatt]

Comment on attachment 2830 [details]
Proposed patch

Excellent. r=me.