1179285 - text-combine-upright should be behind a disabled pref

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

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

Now that we're shipping the pref to enable writing-mode, that currently enables text-combine-upright.  Since it looks (though double-check me, please!) like we have parsing for text-combine-upright but no other support for it, we should switch text-combine-upright to being behind a different pref.  (Should be a one-line patch plus a tweak to property-database.js.)

Comment 1

[Jonathan Kew [:jfkthame]]

You're right, we should keep this disabled until we actually support it.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Give text-combine-upright its own pref, separate from writing-mode (vertical-text), so that we can keep it disabled until we actually have a layout implementation backing the CSS property

Something like this?

Comment 3

[Jonathan Kew [:jfkthame]]

As you'll see, I also removed ALWAYS_ENABLED_IN_UA_SHEETS from text-combine-upright; it got included when that was assigned to the rest of the vertical-text-related properties, but I don't think there's any need for it there.

Comment 4

[Jean-Yves Perrier [:teoli]]

Setting dev-doc-needed as I've added it to Fx 41 for devs and I need to remember to remove it (and somebody else is working on that page right now :-) )

Comment 5

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

Thanks for fixing this so quickly.

Also, don't forget that it will need uplift to Aurora, since we don't want to ship this in 41.

Comment 6

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/643d8e8d61a2

Comment 7

[Sebastian Zartner [:sebo]]

I added a note about this at https://developer.mozilla.org/en-US/docs/Web/CSS/text-combine-upright#Browser_compatibility.

@teoli: As you obviously already removed the property from the Firefox 41 for devs page, I set dev-doc-complete.

Sebastian

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/643d8e8d61a2

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 8628457 [details] [diff] [review]
Give text-combine-upright its own pref, separate from writing-mode (vertical-text), so that we can keep it disabled until we actually have a layout implementation backing the CSS property

Approval Request Comment
[Feature/regressing bug #]: enabling writing-mode (bug 1138384)
[User impact if declined]: the text-combine-upright property will be parsed in CSS and exposed to JS even though not actually implemented in layout
[Describe test coverage new/current, TreeHerder]: covered by layout/style mochitests
[Risks and why]: low risk, just moves the CSS property behind a separate default-false pref
[String/UUID change made/needed]: n/a

Comment 10

[Kate Glazko]

Tracking for 41 because affected.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8628457 [details] [diff] [review]
Give text-combine-upright its own pref, separate from writing-mode (vertical-text), so that we can keep it disabled until we actually have a layout implementation backing the CSS property

Approving for uplift to Aurora. Patch has been in m-c for a few days and it looks safe upon review.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Requesting QE to verify this CSS property is disabled.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/16b1e64fbed3

Comment 14

[Andrei Vaida [:avaida]]

Verified fixed on Firefox 41.0b2 (20150817163452) and Aurora 42.0a2 (2015-08-18), using Windows 7 (x64), Mac OS X 10.10.4 and Ubuntu 14.04 (x86).

I've confirmed that "layout.css.text-combine-upright.enabled" is false by default and that the "text-combine-upright" CSS property is disabled, using this testcase [1]. 


[1] https://www.dropbox.com/s/lswdmjnm95rz9ok/testcase.html?dl=0.