Last Comment Bug 774335 - Implement unicode-bidi: isolate-override (replacing ability to combine isolate and bidi-override keywords)
: Implement unicode-bidi: isolate-override (replacing ability to combine isolat...
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Simon Montagu :smontagu
:
Mentors:
http://dev.w3.org/csswg/css3-writing-...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-16 09:50 PDT by Simon Montagu :smontagu
Modified: 2012-08-14 02:32 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (11.82 KB, patch)
2012-07-17 08:17 PDT, Simon Montagu :smontagu
dbaron: review+
Details | Diff | Review
Changes to tests that depended on the old syntax (2.83 KB, patch)
2012-07-17 08:17 PDT, Simon Montagu :smontagu
dbaron: review+
Details | Diff | Review

Description Simon Montagu :smontagu 2012-07-16 09:50:58 PDT
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 2012-07-17 08:17:12 PDT
Created attachment 642961 [details] [diff] [review]
Patch
Comment 2 Simon Montagu :smontagu 2012-07-17 08:17:49 PDT
Created attachment 642962 [details] [diff] [review]
Changes to tests that depended on the old syntax
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-07-27 15:58:06 PDT
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-7 (review requests must explain patch) 2012-07-27 15:59:10 PDT
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 2012-07-29 21:26:36 PDT
> 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.

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