If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

add 'none' value to font-variant-ligatures

RESOLVED FIXED in mozilla28

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

unspecified
mozilla28
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [DocArea=CSS])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Add support for the 'none' value to font-variant-ligatures.  This will also need to be added to the shorthand once the pref is dropped.
(Assignee)

Updated

4 years ago
Blocks: 913153
(Assignee)

Updated

4 years ago
Blocks: 651693
Keywords: dev-doc-needed
(Assignee)

Comment 1

4 years ago
Created attachment 829938 [details] [diff] [review]
patch, add none value for font-variant-ligatures

Adds the 'none' value to font-variant-ligatures which explicitly disables all ligature features.

Spec reference:
http://dev.w3.org/csswg/css-fonts/#font-variant-ligatures-none-value
Assignee: nobody → jdaggett
Attachment #829938 - Flags: review?(dbaron)
(Assignee)

Comment 2

4 years ago
Created attachment 829939 [details] [diff] [review]
patch, reftest for none value

Add a simple, explicit reftest for this.  (The main patch includes in valid/invalid combinations for font-variant-ligatures).
Attachment #829939 - Flags: review?(jfkthame)
Comment on attachment 829939 [details] [diff] [review]
patch, reftest for none value

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

::: layout/reftests/font-features/reftest.list
@@ +27,5 @@
>  # compare feature specified within @font-face to same feature in style rule
>  HTTP(..) == font-features-hlig-2.html font-features-hlig.html
>  HTTP(..) == font-features-hlig-4.html font-features-hlig.html
>  HTTP(..) != font-features-hlig-5.html font-features-hlig.html
> +HTTP(..) == font-features-ligatures-none.html font-features-noliga.html

Doesn't this need the font-features pref to be enabled? Otherwise it'll fail on release channels, I think.
Comment on attachment 829938 [details] [diff] [review]
patch, add none value for font-variant-ligatures

property_database.js:
You should also have an invalid_values item with "none" as the
first value, but also a second value.

(Maybe the same for 
 layout/reftests/font-features/font-variant-features.js ?)

nsFont.cpp:
It would probably help to have comments for the three items in
ligDefaults that have extra code associated with them below, pointing
to that code.  Also maybe even make all 3 special cases have comments
of the form:
>+      // liga already disabled, need to disable dlig, hlig, calt, clig
like you do for the new one.

>+    if ((val & NS_FONT_VARIANT_LIGATURES_NONE) &&
>+        (val & ~NS_FONT_VARIANT_LIGATURES_NONE)) {

Feels a little safer to write this as either:
+    if ((val & NS_FONT_VARIANT_LIGATURES_NONE) &&
+        (val != NS_FONT_VARIANT_LIGATURES_NONE)) {
or:
+    if ((val & NS_FONT_VARIANT_LIGATURES_NONE) &&
+        (val & ~int32_t(NS_FONT_VARIANT_LIGATURES_NONE))) {
just in case the type of the constant doesn't (in the future) match
 the type of val.

r=dbaron with that
Attachment #829938 - Flags: review?(dbaron) → review+
(Assignee)

Comment 5

4 years ago
Created attachment 8333606 [details] [diff] [review]
patch, reftest for none value

Add pref for font features being enabled.
Attachment #829939 - Attachment is obsolete: true
Attachment #829939 - Flags: review?(jfkthame)
Attachment #8333606 - Flags: review?(jfkthame)
Attachment #8333606 - Flags: review?(jfkthame) → review+
(Assignee)

Comment 6

4 years ago
Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a93c5ec2aea
https://hg.mozilla.org/integration/mozilla-inbound/rev/12f8868df18c
https://hg.mozilla.org/mozilla-central/rev/1a93c5ec2aea
https://hg.mozilla.org/mozilla-central/rev/12f8868df18c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [DocArea=CSS]
Doc up-to-date:
https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-ligatures
https://developer.mozilla.org/en-US/Firefox/Releases/28
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.