1417761 - Remove pref "layout.css.text-combine-upright.enabled"

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Xidorn Quan [:xidorn] UTC+10]

It has been shipped since Firefox 48, and other browsers have similar impls, and the related spec has been in CR since a while ago. The syntax of this property we implemented should be considered to be pretty stable, so I think we can remove this pref.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Having a go at it.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1417761 - Remove "layout.css.text-combine-upright.enabled" pref -

Shipped since Firefox 48, other browsers have similar impls, and the related
spec has been in CR since a while ago.
The syntax of this property as implemented should be considered to be pretty
stable, so we can remove this pref.

Review commit: https://reviewboard.mozilla.org/r/240212/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240212/

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8971464 [details]
Bug 1417761 - Remove "layout.css.text-combine-upright.enabled" pref -

https://reviewboard.mozilla.org/r/240212/#review246022

::: modules/libpref/init/all.js:2902
(Diff revision 1)
>  // Is support for CSS text-combine-upright: digits 2-4 enabled?
>  pref("layout.css.text-combine-upright-digits.enabled", false);

This pref is no longer effective because stylo doesn't include this. Could you add a followup patch to remove this pref and any related code as well?

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you for the lightning-quick review Xidorn.


(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> >  // Is support for CSS text-combine-upright: digits 2-4 enabled?
> >  pref("layout.css.text-combine-upright-digits.enabled", false);
> This pref is no longer effective because stylo doesn't include this. Could
> you add a followup patch to remove this pref and any related code as well?

No chance this may instead be re-implemented in Stylo one day?

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ccb42452660144b8709709945f6ee143648fc63

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Gerald Squelart [:gerald] from comment #4)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> > >  // Is support for CSS text-combine-upright: digits 2-4 enabled?
> > >  pref("layout.css.text-combine-upright-digits.enabled", false);
> > This pref is no longer effective because stylo doesn't include this. Could
> > you add a followup patch to remove this pref and any related code as well?
> 
> No chance this may instead be re-implemented in Stylo one day?

Implementing the style part is easy, but the layout part is hard, so we are not going to revive its style part until we decide to work on the layout part.

Actually the usefulness of "digits" value as it's currently defined is unclear either, given it's easy to cause undesired result if not used carefully. Currently only Edge implemented that, and I don't think Chrome has plan for this at the moment either.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e502da324b70
Remove "layout.css.text-combine-upright.enabled" pref - r=xidorn

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e502da324b70