[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
(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.
Comment on attachment 2830 [details] Proposed patch Excellent. r=me.