In a R EN L sequence of runs, the EN should have the highest embedding level. As of bidi.cpp r1.127, if the L is not explicit but only implicit from context, the EN ends up with the lower embedding level, and the layout order is wrong. Expected layout for the testcase is the digit 1 followed by Hebrew letter Aleph in all 3 cases (followed by nothing, a full stop or the letter Z). Firefox renders as expected. Actual layout with TOT is Hebrew letter Aleph followed by 1 (followed by nothing or a full stop) in the first two cases, and as expected in the last case.
Created attachment 2654 [details] Testcase
Created attachment 2657 [details] A patch This fixes the regression. The patch makes 14 of the layout tests fail, because it splits a few runs, but they render the same with it. Still, it may be possible to avoid this. There's also room for improvement in moving out stuff that is repeated in both "if" and "else" cases (left that way for the sake of explicitness while I'm working on it), so I'm not even submitting it for review at this stage.
confirmed regression, moving to p1.
(In reply to comment #3) > confirmed regression, moving to p1. Hm... I don't know how this priority thing works (or how anything works for that matter), so I'll just state that 1. If the choice is between r1.127 and r1.126, then despite the regression, r1.127 is "less buggy", i.e. the regression is far less likely to occur in everyday use than the problem r1.127 fixes. 2. I don't think that those 14 layout tests failing is significant. 3. There are a few other bidi layout issues in TOT (which are not regressions). I'm working on identifying and fixing them. I'll submit a bug about them when I give up on fixing them all or think that I've done it.
attachment 2657 [details] also breaks this: <p dir="rtl"> for 123 abc </p> Don't bother looking at it.
Created attachment 2659 [details] A better patch This patch corrects the problem mentioned in comment #5. It also fixes the layout of <p dir="rtl">א1.</p> and of the following, which were broken even before r1.127: <p dir="rtl">123a אבג</p> <p dir="rtl">123a+ אבג</p> <p dir="rtl">אבג 1+5.2x, דהו</p> This patch changes the layout of <p dir="rtl">def ١٢٣ abc</p>, which was broken anyway. I don't know if it's more severely broken now. Other cases involving Arabic numbers are broken just the same with or without this patch. Finally, this patch fails 137(!) layout tests, because now instead of splitting a few runs, it merges many runs. I haven't gone through all 137 of them yet, but so far it seems that the failures are only a result of run merges (and still a few run splits). These do not affect the layout but sometimes slightly affect rendering (due to rounding and such, I presume). In general, I suppose long runs are better, but it's just a daunting task to verify that all 137 failures are benign. Otherwise, the part that's responsible for them can be left out (it's the different handling of the L after EN case), and then there are far fewer failures, all of which are splits.
Attachment 2668 [details] to bug 3733 addresses this bug.
Fixed along with bug 3733.