Last Comment Bug 706194 - Adapt and check in tests from webkit for new unicode-bidi attributes
: Adapt and check in tests from webkit for new unicode-bidi attributes
: rtl
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Simon Montagu :smontagu
Depends on: 713621 755244
  Show dependency treegraph
Reported: 2011-11-29 11:29 PST by Simon Montagu :smontagu
Modified: 2012-05-15 05:32 PDT (History)
3 users (show)
smontagu: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch part 1 (20.35 KB, patch)
2011-12-27 00:03 PST, Simon Montagu :smontagu
roc: review+
Details | Diff | Splinter Review
Patch part 2 (10.27 KB, patch)
2011-12-27 00:05 PST, Simon Montagu :smontagu
roc: review+
Details | Diff | Splinter Review
Patch part 3 (2.03 KB, patch)
2012-03-06 13:06 PST, Simon Montagu :smontagu
dbaron: review+
Details | Diff | Splinter Review

Comment 1 Simon Montagu :smontagu 2011-12-27 00:03:21 PST
Created attachment 584392 [details] [diff] [review]
Patch part 1

For clarity I'll do this in 2 parts: this is the initial adaptation of the webkit tests to our test harnesses, which has lots of failures.
Comment 2 Simon Montagu :smontagu 2011-12-27 00:05:47 PST
Created attachment 584394 [details] [diff] [review]
Patch part 2

... and these are the changes to make the tests pass (modulo bugs noted in comments)
Comment 3 Simon Montagu :smontagu 2011-12-27 00:33:08 PST
Comment on attachment 584394 [details] [diff] [review]
Patch part 2

Reasons for the changes:

>+    text-align: left;
 in a number of tests, because of a difference between the way that we and webkit handle unicode-bidi: plaintext. If no text-align is specified, we align paragraphs to the left and right depending on the resolved direction of each paragraph, but webkit aligns everything depending on the direction of the enclosing element. See and

>diff --git a/layout/reftests/bidi/unicode-bidi-isolate-basic.js b/layout/reftests/bidi/unicode-bidi-isolate-basic.js

>-    var strongRTLs = ['ש', 'נ', 'ב', 'ג', 'ק', 'כ', 'ע'];
>+    var strongRTLs = ['א', 'ב', 'ג', 'ד', 'ה', 'ו', 'ז'];

Using Hebrew letters in alphabetical order makes the test results easier to read for me (the original used the same key positions as a,b,c,d,e,f,g)

>diff --git a/layout/style/test/test_default_bidi_css.html b/layout/style/test/test_default_bidi_css.html

>-    ['div', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['div', {'dir': 'ltr'}, 'ltr', '-moz-isolate'],
>+    ['div', {'dir': 'rtl'}, 'rtl', '-moz-isolate'],
>     ['div', {'dir': 'auto'}, 'ltr', '-moz-isolate'],
>-    ['div', {'dir': ''}, 'ltr', 'embed'],
>+    ['div', {'dir': ''}, 'ltr', '-moz-isolate'],

>-    ['pre', {}, 'ltr', 'normal'],
>-    ['pre', {'dir': 'ltr'}, 'ltr', 'embed'],
>-    ['pre', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['pre', {}, 'ltr', '-moz-isolate'],
>+    ['pre', {'dir': 'ltr'}, 'ltr', '-moz-isolate'],
>+    ['pre', {'dir': 'rtl'}, 'rtl', '-moz-isolate'],
>     ['pre', {'dir': 'auto'}, 'ltr', '-moz-plaintext'],
>-    ['pre', {'dir': ''}, 'ltr', 'embed'],
>+    ['pre', {'dir': ''}, 'ltr', '-moz-isolate'],

Another difference between us and webkit: we give block elements with a dir attribute -moz-isolate, not embed ( and
Comment 4 Simon Montagu :smontagu 2012-03-06 13:06:03 PST
Created attachment 603424 [details] [diff] [review]
Patch part 3

Assuming that we want to WONTFIX bug 713621, this makes the necessary changes to test_default_bidi_css.html
Comment 5 David Baron :dbaron: ⌚️UTC-8 2012-04-22 16:31:31 PDT
Comment on attachment 603424 [details] [diff] [review]
Patch part 3

Comment 7 Ed Morley [:emorley] 2012-04-23 02:52:52 PDT
Sorry, backed out ( for what looked like bustage, but was slightly ahead of myself (and clearly still half asleep!) as was just hg issues. 

Comment 8 Ed Morley [:emorley] 2012-04-23 03:54:20 PDT
Followup to fix bad qimport where the non-ASCII text in the tests got mangled :-(

(Having a successful morning here!)

Note You need to log in before you can comment on or make changes to this bug.