See bug 1221034 comment 2 for the description of the issue.

Review commit: https://reviewboard.mozilla.org/r/60490/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/60490/

The code changes in this patch are from the ICU code at the following places correspondingly: https://dxr.mozilla.org/mozilla-central/rev/d1102663db10b3d4b9358f3cf4e16b7c56902352/intl/icu/source/common/ubidi.c#2752-2754 https://dxr.mozilla.org/mozilla-central/rev/d1102663db10b3d4b9358f3cf4e16b7c56902352/intl/icu/source/common/ubidi.c#2161-2163 https://dxr.mozilla.org/mozilla-central/rev/d1102663db10b3d4b9358f3cf4e16b7c56902352/intl/icu/source/common/ubidi.c#2243-2244

With this and bug 1160847, we pass all unicode-bidi tests in the csswg repo.

try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c87e83b69d9d I accdentally triggered reftests for all platforms...

Comment on attachment 8764807 [details] Bug 1281988 - Skip BN characters when resolving implicit levels in bidi engine. https://reviewboard.mozilla.org/r/60490/#review57672 r=me, but please make the loops a little less cryptic so that future readers don't worry they're a mistake. :) ::: layout/base/nsBidi.cpp:1526 (Diff revision 1) > > for (i = aStart; i <= aLimit; i++) { > if (i >= aLimit) { > - if (aLimit > aStart) { > - dirProp = mDirProps[aLimit - 1]; > + int32_t k; > + for (k = aLimit - 1; > + k > aStart && (DIRPROP_FLAG(dirProps[k]) & MASK_BN_EXPLICIT); k--); Please move the semicolon that forms the (empty) loop body here to a separate line, perhaps with a comment to help readability, e.g.: /\* empty loop body \*/ ; Actually, I suppose it's arguable that Gecko style would expect braces around the body, even if it's empty. So maybe: for (...) { // empty loop body } is stylistically more consistent. ::: layout/base/nsBidi.cpp:1573 (Diff revision 1) > } > } > } > > - dirProp = dirProps[aLimit - 1]; > + for (i = aLimit - 1; > + i > aStart && (DIRPROP_FLAG(dirProps[i]) & MASK_BN_EXPLICIT); i--); As above.

https://reviewboard.mozilla.org/r/60490/#review57672 > Please move the semicolon that forms the (empty) loop body here to a separate line, perhaps with a comment to help readability, e.g.: > > /\* empty loop body \*/ ; > > Actually, I suppose it's arguable that Gecko style would expect braces around the body, even if it's empty. So maybe: > > for (...) { > // empty loop body > } > > is stylistically more consistent. Actually I intentionally did so to make it look more similiar to the corresponding ICU code. It seems to me the whole nsBidi doesn't really follow the Gecko style, which made me think that there is some other rules for this file. But I'm fine to improve the readability.

Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/23a3e2678c02 Skip BN characters when resolving implicit levels in bidi engine. r=jfkthame