The HTML5 spec changes the semantics of the dir attribute to use unicode-bidi:isolate instead of unicode-bidi:embed (on all elements except BDO), and unicode-bidi: isolate-override instead of unicode-bidi:bidi-override on BDO. Here is a link to the relevant section of the HTML5 spec and the CSS involved: http://www.w3.org/TR/html5/rendering.html#bidi-rendering Similarly, the definition of the dir attribute in the HTML5 spec speaks of it making the element's content "directionally isolated"; see http://www.w3.org/TR/html5/dom.html#the-dir-attribute This is a very important change that should make it easier to author bidi pages. It was proposed and extensively discussed by the W3C i18n WG (http://www.w3.org/International/wiki/Html-bidi-isolation), and then by the WHATWG and HTML5 WG (https://www.w3.org/Bugs/Public/show_bug.cgi?id=23260). It is also a very easily implemented change - just a change in the default CSS. The change is not backward compatible, but it is very close to the behavior of IE 8 through IE 11. The bodies mentioned above were well aware of the lack of backward compatibility, but approved the change nonetheless because of its benefits going forward. It is now the standard. Firefox's implementation of unicode-bidi:isolate (prefixed as -moz-isolate) has at least passed all tests in l18n wg. But we do still have some issues with isolate-override currently. Related issue for Chrome: https://code.google.com/p/chromium/issues/detail?id=391260 Related issue for WebKit: https://bugs.webkit.org/show_bug.cgi?id=134630 Today, I discussed with Richard Ishida (from L18N WG), Koji Ishii (from Blink team), and Myles C. Maxfield (from WebKit team). No one had idea how many pages would be broken, and we didn't even have idea how this breakage could be measured. But we thought this is a good change we probably should go forward.

:jfkthame, do you think we are fine with this change? Blink is going to try shipping this change first if we agree with it.

I think the change makes sense in principle; the only concern would be the (unknown) scale of the impact on existing content. But as per discussion in the Blink bug (e.g. https://code.google.com/p/chromium/issues/detail?id=391260#c3) it seems worth trying to move forward. I'd like to hear Simon's opinion, too. We'll need a set of testcases. (Maybe from i18n WG.)

(In reply to Jonathan Kew (:jfkthame) from comment #2) > We'll need a set of testcases. (Maybe from i18n WG.) Yeah. Koji told me he had got a set of tests from Richard, and I guess we could put them to web-platform-tests later.

PR for web-platform-tests: https://github.com/w3c/web-platform-tests/pull/2357 those tests need a reviewer you can see the results of the tests for major browsers at http://www.w3.org/International/tests/repo/results/the-dir-attribute-isolation (the first table shows current results for released versions of browsers; the second shows likely results for updated browsers, based on a CSS shim recommended for content authors)

I believe this is the right thing to do.

Do you want to do this, then?

Given jfkthame and smontagu both agree with it, and jet also thought it's fine to move forward, I think we indeed want to do this. Koji has been working on this change on Blink side, and it seems he just found it is not as easy as a single line stylesheet change like we thought. There are some issues around ISO-8859-8 and editing and selection for them. I'd like to wait a bit and see what's going on there first anyway.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #7) > Koji has been working on this change on Blink side, and it seems he just > found it is not as easy as a single line stylesheet change like we thought. > There are some issues around ISO-8859-8 and editing and selection for them. Do you have more details on this? I'd like to understand the issues here.

I'm summarizing my investigations here: https://docs.google.com/document/d/1Ojb95CQJ49aqmJ6SKVOMd_rNTGx8e0LEqjMFJBM1dWI/edit It'd be great if you could review and comments if any.

i just added some comments to https://docs.google.com/document/d/1Ojb95CQJ49aqmJ6SKVOMd_rNTGx8e0LEqjMFJBM1dWI/edit

So the simple UA stylesheet change breaks tests from bug 83958, bug 263359, bug 613157, and bug 698706.

The test from bug 83958 is intended to test bidi embedding. It comes in three variants (-a, -b, -c), one using control codes, one using CSS, and one using inlines with dir. If we make this change then the third variant is by definition no longer testing what it was intended to test, so we can just delete it with no harm done. Bug 613157 and bug 698706 both show a difference between embedding and isolation in the ordering of successive spans whose direction is opposite to the base direction. This is expected and the references should be fixed accordingly. Both tests are still testing what they were intended to test. (Bug 698706 is particularly interesting, because the testcase was reduced from actual web content on Facebook, so it might point to a case where this change would cause a visible regression. Only "might" because (a) the content in question was rather psychotic, due to partial localization of the UI, and that exact construct doesn't seem to occur any more on FB (though a parallel case might still come up), and (b) it's a hard judgment call whether the change is a regression or an improvement, and I could argue either way.) I'm not sure what the status of bug 263359 is: the visible difference is in the positioning of spaces at the end of <span>s. I don't know why the change from embedding to isolation causes this difference, without a detailed analysis that I don't have time to perform right now, so this may or may not be exposing a bug.

Re the reference to bug 263359 in comment 13 ("the visible difference is in the positioning of spaces at the end of <span>s"): I think that you might have the wrong bug number there. Bug 263359 does not have spans, deals with the bidi properties of a newline in <pre>, and as far as I know can't have anything to do with dir being isolating.

No, if you look at the actual test[1] you'll see that it does have <span>s within the test paragraph with 'white-space:pre'; those spans have trailing spaces; and the test failure[2] looks like it's related to those trailing spaces being treated differently. One possibly-interesting point is that there are three (nested) spans in the test, and the problem occurs at the end of each of them (three lines in the result are affected); but only one of those spans actually has a 'dir' attribute. (The other two have the CSS 'direction' property.) [1] http://hg.mozilla.org/mozilla-central/annotate/85cf2e720a84/layout/reftests/bidi/263359-3.html [2] http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://archive.mozilla.org/pub/firefox/try-builds/xquan@mozilla.com-b6a25ef29d71e7e312068e47516c8c393a31f52a/try-linux-debug/try_ubuntu32_vm-debug_test-reftest-1-bm05-tests1-linux32-build1604.txt.gz&only_show_unexpected=1

Right, sorry about that! Looking at the test, I think we should start by focusing on the display of line 31, which is the simplest case (being outside of the unicode-bidi:bidi-override spans). IMO, according to the UBA, the space should be displayed at the right end of the line, not the left end as is currently the case when dir uses unicode-bidi:isolate. My reasoning: - CSS writing modes level 3 says: "[unicode-bidi:isolate] corresponds to inserting an LRI (U+2066), for direction: ltr, or RLI (U+2067), for direction: rtl, at the start of the inline box, and a PDI (U+2069) at the end of the box." - Thus, line 31 should be treated as if a PDI follows the space, just before the </span>. - A newline inside a <pre> constitutes a bidi paragraph break. - Thus, line 31 (like any other line inside the pre) is a separate bidi paragraph - CSS writing modes level 3 says: "the paragraph embedding level must be set [...] according to the direction property of the paragraph’s containing block" - Thus, line 31's paragraph embedding level is 0 (i.e. LTR), even though it is inside the <span dir="rtl">, because its containing block is the <p> above that, which has the default direction:ltr, and thus embedding level 0. - UBA's L1 says: "On each line, reset the embedding level of the following characters to the paragraph embedding level: [...] Any sequence of whitespace characters and/or isolate formatting characters (FSI, LRI, RLI, and PDI) preceding a segment separator or paragraph separator". - Thus, line 31's trailing space and the PDI should be reset to the paragraph embedding level, which we determined above as 0. This should put them both at the right end of the line. Incidentally, this would also be the case if line 31 did not have the </span> at its end, but simply ended in a newline: - CSS writing modes level 3 says: "If an inline box is broken around a bidi paragraph boundary (e.g. if split by a block or forced paragraph break), then the HL3 bidi control codes assigned to the end of the box are also added before the interruption and the codes assigned to the start of the box are also added after it. (In other words, any embedding levels, isolates, or overrides started by the box are closed at the paragraph break and reopened on the other side of it.)" - Thus, line 31 should still be treated as if its trailing space were followed by a PDI - As before, by L1, embedding level of the space and the PDI should get reset to 0, putting them on the right end of the line. Thus, I believe that Mozilla has a bug. To reproduce this issue, it is unnecessary to put the unicode-bidi:isolate on a span inside a <pre>. You can put it right on the <pre>. The test case: data:text/html,<pre style="unicode-bidi:-moz-isolate; direction:rtl; text-align:left">x </pre> I believe that according to UBA's L1, the x should be displayed flush with the left margin, i.e. with the spaces displayed on the right end of the line. What I see right now is the spaces to the left of the x. I think that it would be best to fix this bug before making unicode-bidi:isolate the default for the dir attribute.

Sorry, I was wrong about the test case for the bug. The test case I gave, data:text/html,<pre style="unicode-bidi:-moz-isolate; direction:rtl; text-align:left">x </pre> is actually displayed as it should be, since the containing block of the paragraph is the <pre> itself, and it has direction:rtl, so indeed the spaces are supposed to wind up on the left. The correct test case is: data:text/html,<pre><span style="unicode-bidi:-moz-isolate; direction:rtl">x </span></pre> or even better: data:text/html,<pre><span style="unicode-bidi:-moz-isolate; direction:rtl">x %0Dy %0Dz </span></pre>

FYI, Chrome has the same bug. I filed https://code.google.com/p/chromium/issues/detail?id=565329.

I have filed issue 1230455 for this. BTW, the Chrome bug reproduces equally well for unicode-bidi:embed, since Chrome generally speaking does not support UBA's L1. This is not the case in Mozilla.

Blink has landed this change for around a month with no bug reported yet. It seems currently the only blocker for Gecko here is the test of bug 263359 which would need a non-trivial work from bug 1160847 to fix. As this is a potential breaking change which needs coordination between browsers, I hope we can also ship this change as soon as possible. smontagu, jfkthame, are you fine with landing this change with that test marked as expected-fail? If you are fine with that, I'll submit a patch to do so.

Personally, I'm finding it hard to make up my mind here... on the one hand, I'm reluctant to land a change that will regress existing correct behavior (as in the 263359-3.html testcase); but OTOH given that webkit/blink already have the same buggy behavior, it seems unlikely there's much real content that would be badly affected, so perhaps we shouldn't worry too much. I'd like to hear what Simon thinks, if he has a strong view one way or the other. (Ideally, of course, we'd go ahead and fix bug 1160847, but that looks like it may not be quick or easy.)

I think I'm OK with landing the change and marking the test as failing, with a pointer to bug 1230455. It might also be worth considering and investigating whether checking in the "w-i-p" patch in bug 1160847 comment 1 (attachment 8605760 [details] [diff] [review]), with a little more work, would be a good tactic: if it fixes the regressions from this checkin, and doesn't cause any other regressions, then we would be better off all round than the status quo, even though it doesn't fix bug 717811 as it was supposed to.

I guess I found a bug of Bugzilla :) superreview is not blocked by "Block review and feedback requests" option.

Comment on attachment 8707765 [details] [diff] [review] patch Is there a bug on unprefixing -moz-isolate (and -moz-plaintext -- maybe one bug or two)? Are those things we can do yet? There should be a bug or two whether or not it's something we can do now. And how hard is it to still test what 263359-3 was testing? I didn't look over the test changes all that closely -- maybe Jonathan wanted to do that more closely -- or maybe it's not needed?

(In reply to David Baron [:dbaron] ⌚️UTC+13 from comment #26) > Comment on attachment 8707765 [details] [diff] [review] > patch > > Is there a bug on unprefixing -moz-isolate (and -moz-plaintext -- maybe one > bug or two)? Are those things we can do yet? There should be a bug or two > whether or not it's something we can do now. It is bug 1141895. We haven't passed all tests on those values yet, since we are on the halfway of updating our UBA impl. See bug 1221034 comment 1. But I guess we can unprefix -moz-isolate first, since we've passed all tests around this value. > And how hard is it to still test what 263359-3 was testing? Not sure what that test means to test apart from the failing part (the tailing whitespaces). We may just comment the two lines out to make it continue testing other parts. > I didn't look over the test changes all that closely -- maybe Jonathan > wanted to do that more closely -- or maybe it's not needed? I hope someone could review the references I changed, since I'm not pretty confident on what is the best way to simulate bidi behavior there.

Comment on attachment 8707765 [details] [diff] [review] patch Review of attachment 8707765 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/reftests/bidi/613157-2-ref.html @@ +4,5 @@ > <meta charset="utf-8"> > <title>Inline blocks shouldn't end the paragraph</title> > </head> > <body> > + <p>&#x202e;אני אוהב&#x202c;--&gt;Firefox 4 --&gt;8 ימים בשבוע</p> In general, I'd favor forcing all the text to a single direction for the reference case (i.e. staying with the existing pattern of just a single directional override that applies to the entire line), and rearrange the content as necessary into the new expected visual order. Let's try to keep the use of mixtures of overrides, multiple direction runs (with their potential boundary issues, etc) for the testcases; keep the references pure unidirectional. This may not generally be feasible for testcases that involve line wrapping, such as the 698706-1 example that follows; but for single-line examples like this, I'd prefer it.

Comment on attachment 8719633 [details] [diff] [review] patch, r=dbaron Review of attachment 8719633 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, I think that makes the reference simpler/more robust.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cd67dd1dfee8347d3ea7d7141de6306a0b60dbf Bug 1218706 - Make 'unicode-bidi: isolate' the default for elements with a dir attribute. r=dbaron,jfkthame

https://hg.mozilla.org/integration/mozilla-inbound/rev/c8c3a62923116cc392fe96f56c5db7841fba02a5 Bug 1218706 followup - Mark some dir-isolation web-platform test expected pass.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c05368b0d0e0d9ce1272233569c8c417ec7cb18 Bug 1218706 followup 2 - Mark some dir-isolation web-platform tests expected-fail on Win7.

https://hg.mozilla.org/mozilla-central/rev/1cd67dd1dfee https://hg.mozilla.org/mozilla-central/rev/c8c3a6292311 https://hg.mozilla.org/mozilla-central/rev/6c05368b0d0e

html.css still reads > bdo, bdo[dir] { unicode-bidi: bidi-override; } but the spec says > bdo, bdo[dir] { unicode-bidi: isolate-override; } as :xidorn explained in comment 0. Is it okay?

This could be an issue... Filed bug 1221034 for that. I think we need to finish support of isolate-override before we start using it there.

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/elements-with-dir-attribute-will-have-unicode-bidi-isolate/

I'm hitting unexpected pass on Windows 7 with Skia content for these tests. Do you remember why these were marked as expected fail in part 2? Thanks!

I think that was just some quick fix to make the tree green. I don't know why the tests fail on Win7 since there was no screenshot for wpt reftests. But it is possible that switching graphics backend fixes that if it failed just because of some fuzzy.