774335 - Implement unicode-bidi: isolate-override (replacing ability to combine isolate and bidi-override keywords)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Simon Montagu :smontagu]

The latest version of CSS3 Writing Modes has a new value "isolate-override" for unicode-bidi, instead of the combination "bidi-override isolate".

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

Comment 2

[Simon Montagu :smontagu]

Attachment: Changes to tests that depended on the old syntax

Comment 3

[Simon Montagu :smontagu]

I'll move the hunk at https://bugzilla.mozilla.org/attachment.cgi?id=642962&action=diff#a/layout/style/nsCSSPropList.h_sec1 into attachment 642961 [details] [diff] [review]

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 642961 [details] [diff] [review]
Patch

>diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h

> CSS_PROP_TEXTRESET(
>     unicode-bidi,
>     unicode_bidi,
>     UnicodeBidi,
>-    CSS_PROPERTY_PARSE_VALUE |
>-        CSS_PROPERTY_VALUE_PARSER_FUNCTION,
>-    "",
>-    0,
>+    CSS_PROPERTY_PARSE_VALUE,
>+    VARIANT_HK,
>     kUnicodeBidiKTable,
>     CSS_PROP_NO_OFFSET,
>     eStyleAnimType_None)

This looks like a bad merge that didn't compile; you need to put the "" back in.

r=dbaron with that

We should also think about a path to unprefixing this sooner rather than later...

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 642962 [details] [diff] [review]
Changes to tests that depended on the old syntax

>diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h

> CSS_PROP_TEXTRESET(
>     unicode-bidi,
>     unicode_bidi,
>     UnicodeBidi,
>     CSS_PROPERTY_PARSE_VALUE,
>+    "",
>     VARIANT_HK,
>     kUnicodeBidiKTable,
>     CSS_PROP_NO_OFFSET,
>     eStyleAnimType_None)
> CSS_PROP_USERINTERFACE(
>     -moz-user-focus,
>     user_focus,
>     CSS_PROP_DOMPROP_PREFIXED(UserFocus),

Aha.  This merge got in the wrong patch.  You should either put it in the other one or just squash the two patches together.

r=dbaron

Comment 6

[fantasai]

> We should also think about a path to unprefixing this sooner rather than later...

Authors shouldn't be using 'unicode-bidi' in general, so I don't see that as important. The values there mostly exist to support the relevant HTML features.

Comment 7

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f224e55d00b
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1f0011fdef2

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6f224e55d00b
https://hg.mozilla.org/mozilla-central/rev/d1f0011fdef2

Comment 9

[Jean-Yves Perrier [:teoli]]

Updated:
https://developer.mozilla.org/en-US/docs/CSS/unicode-bidi
https://developer.mozilla.org/en-US/docs/Firefox_17_for_developers