1230478 - [css-grid][css-align]Rename 'true' to 'unsafe'

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[yisibl]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.86 Safari/537.36

Steps to reproduce:

CSS spec rename 'true' to 'unsafe'

diff: https://hg.csswg.org/drafts/rev/f34a2a8b8177
spce: https://drafts.csswg.org/css-align/?c=M%3BO%3DD#overflow-values

Comment 1

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

Mats, do you want to look at this?

Comment 2

[Mats Palmgren (inactive)]

I'm a bit busy with other stuff right now, perhaps Tobias have time for it?

Comment 3

[yisibl]

Blink fixed: https://code.google.com/p/chromium/issues/detail?id=565883

Comment 4

[yisibl]

WebKit done: http://trac.webkit.org/changeset/194104

Comment 5

[Mats Palmgren (inactive)]

Attachment: part 1 - [css-grid][css-align] Rename 'true' to 'unsafe' in <overflow-position>.

Comment 6

[Mats Palmgren (inactive)]

Attachment: part 2 - [css-grid][css-align] Change 'true' to 'unsafe' in tests

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb24ce24d3b6

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8702162 [details] [diff] [review]
part 2 - [css-grid][css-align] Change 'true' to 'unsafe' in tests

Review of attachment 8702162 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/property_database.js
@@ +4244,1 @@
>                        "stretch safe" ]

Maybe worth adding a formerly-valid "true" value to the invalid_values list here?

e.g. "true end"

(This proves/checks that we really don't support the "true" keyword here anymore.)

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8702161 [details] [diff] [review]
part 1 - [css-grid][css-align] Rename 'true' to 'unsafe' in <overflow-position>.

Review of attachment 8702161 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/nsStyleConsts.h
@@ +487,5 @@
>  #define NS_STYLE_ALIGN_SELF_END         12
>  #define NS_STYLE_ALIGN_SPACE_BETWEEN    13
>  #define NS_STYLE_ALIGN_SPACE_AROUND     14
>  #define NS_STYLE_ALIGN_SPACE_EVENLY     15
>  #define NS_STYLE_ALIGN_LEGACY        0x10 // mutually exclusive w. SAFE & TRUE

Actually, one nit on part 1 -- this comment needs s/TRUE/UNSAFE/

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9403494cc755
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5245c7bbe0f

Comment 10

[Mats Palmgren (inactive)]

(In reply to Daniel Holbert [:dholbert] from comment #7)
> Maybe worth adding a formerly-valid "true" value to the invalid_values list
> here?

Good idea, but I didn't bother because I think we should just remove
the 'true' keyword instead.  AFAIK, we (now) only use it for text-align[-last]
which dropped 'true' some time ago (and we never enabled it by default).

I filed bug 1237059 about this.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9403494cc755
https://hg.mozilla.org/mozilla-central/rev/d5245c7bbe0f