Last Comment Bug 1119062 - (enable-unicode-range-release) enable unicode-range in release builds
(enable-unicode-range-release)
: enable unicode-range in release builds
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Graphics: Text (show other bugs)
: Trunk
: All All
P2 major with 5 votes (vote)
: mozilla45
Assigned To: John Daggett (:jtd)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 1230437 1253542 unicode-range 1056479 1180560 1218123 1245811 1286011
Blocks: css3-fonts
  Show dependency treegraph
 
Reported: 2015-01-07 17:07 PST by John Daggett (:jtd)
Modified: 2016-07-11 11:34 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
patch, enable unicode-range in release builds (4.30 KB, patch)
2015-01-07 18:01 PST, John Daggett (:jtd)
no flags Details | Diff | Splinter Review
patch, enable unicode-range in release builds (5.17 KB, patch)
2015-11-18 17:58 PST, John Daggett (:jtd)
dbaron: review+
rkothari: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
patch, enable unicode-range load test under Linux (1.37 KB, patch)
2015-11-19 21:31 PST, John Daggett (:jtd)
dbaron: review+
rkothari: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image John Daggett (:jtd) 2015-01-07 17:07:18 PST
The unicode-range implementation is currently only enabled for nightly/aurora builds, waiting for the Linux platform fontlist implementation (bug 1056479). Once that lands and appears to be stable we can enable unicode-range in release builds.
Comment 1 User image John Daggett (:jtd) 2015-01-07 18:01:33 PST
Created attachment 8545607 [details] [diff] [review]
patch, enable unicode-range in release builds
Comment 2 User image dhendo 2015-08-28 08:31:13 PDT
What's the plan for releasing this fix?
Comment 3 User image John Daggett (:jtd) 2015-08-30 22:19:05 PDT
Currently gated by bugs blocking the fontconfig platform fontlist use under Linux. See bug 1180560. That bug gets fixed ==> this bug gets resolved, unicode-range is enabled by default for release builds.
Comment 4 User image John Daggett (:jtd) 2015-11-18 17:58:26 PST
Created attachment 8689290 [details] [diff] [review]
patch, enable unicode-range in release builds

Once bug 1180560 lands we can enable unicode-range in release builds.
Comment 5 User image David Baron :dbaron: ⌚️UTC-8 2015-11-18 21:32:49 PST
Did you send an intent to ship notice to dev-platform?  See https://wiki.mozilla.org/WebAPI/ExposureGuidelines
Comment 7 User image David Baron :dbaron: ⌚️UTC-8 2015-11-19 20:52:14 PST
Comment on attachment 8689290 [details] [diff] [review]
patch, enable unicode-range in release builds

r=dbaron


One other thought, though (since you made me look at what the test coverage is):  given that the reason for unicode-range to exist is that it allows a way of not bothering to download a font at all based on character ranges, it seems like it's probably worth adding a mochitest to actually test the "lack of download" aspect of it (with a base case that should be downloaded).  It shouldn't be too hard to do in the manner of test_bug517224.html and bug517224.sjs in layout/style/test/ (although you probably want multiple tests, like the somewhat more complicated visited_image_loading test in the same directory).
Comment 8 User image John Daggett (:jtd) 2015-11-19 20:58:30 PST
(In reply to David Baron [:dbaron] ⌚UTC-8 from comment #7)
> Comment on attachment 8689290 [details] [diff] [review]
> patch, enable unicode-range in release builds
> 
> r=dbaron
> 
> 
> One other thought, though (since you made me look at what the test coverage
> is):  given that the reason for unicode-range to exist is that it allows a
> way of not bothering to download a font at all based on character ranges, it
> seems like it's probably worth adding a mochitest to actually test the "lack
> of download" aspect of it (with a base case that should be downloaded).  It
> shouldn't be too hard to do in the manner of test_bug517224.html and
> bug517224.sjs in layout/style/test/ (although you probably want multiple
> tests, like the somewhat more complicated visited_image_loading test in the
> same directory).

Hmmm, that's basically what the slew of tests in layout/style/test/test_unicode_range_loading.html are doing? Did you look through those? Would you consider those not sufficient? These tests use the font loading API rather than .sjs magic. But if needed I'm sure I could come up with the kind of test you're thinking about too.
Comment 9 User image John Daggett (:jtd) 2015-11-19 21:31:31 PST
Created attachment 8689912 [details] [diff] [review]
patch, enable unicode-range load test under Linux

Forgot to remove the "skip on Linux" of the unicode-range load test.
Comment 12 User image John Daggett (:jtd) 2015-11-27 00:14:40 PST
Comment on attachment 8689290 [details] [diff] [review]
patch, enable unicode-range in release builds

Approval Request Comment
[Feature/regressing bug #]: With the new work on the Linux font backend, we can now enable unicode-range for beta/release across all platforms.
[User impact if declined]: unicode-range support can't be enabled until FF45
[Describe test coverage new/current, TreeHerder]: landed on trunk last week, no issues reported
[Risks and why]: new fontconfig font backend has been enabled for nightly/aurora for several release cycles now and all known regressions have been resolved
[String/UUID change made/needed]: none
Comment 13 User image John Daggett (:jtd) 2015-11-27 00:15:15 PST
Comment on attachment 8689912 [details] [diff] [review]
patch, enable unicode-range load test under Linux

Approval Request Comment

-- same as above --
Comment 14 User image Ritu Kothari (:ritu) 2015-11-30 15:46:43 PST
John, while reviewing the patch I noticed that the pref "layout.css.unicode-range.enabled" is being removed completely. Is that intentional? I got confused because the bug talks about enabling unicode-range. Does it mean it is on and not something that can be turned off by a pref setting? Please let me know.
Comment 15 User image John Daggett (:jtd) 2015-11-30 15:53:54 PST
(In reply to Ritu Kothari (:ritu) from comment #14)
> John, while reviewing the patch I noticed that the pref
> "layout.css.unicode-range.enabled" is being removed completely. Is that
> intentional? I got confused because the bug talks about enabling
> unicode-range. Does it mean it is on and not something that can be turned
> off by a pref setting? Please let me know.

Right, "enabling" in this case means making it always on by default and removing the pref completely.
Comment 16 User image Ritu Kothari (:ritu) 2015-12-01 11:04:03 PST
Comment on attachment 8689290 [details] [diff] [review]
patch, enable unicode-range in release builds

This has been on Nightly for 10 days or so. Let's uplift to Aurora44.
Comment 17 User image Ritu Kothari (:ritu) 2015-12-01 11:04:33 PST
Comment on attachment 8689912 [details] [diff] [review]
patch, enable unicode-range load test under Linux

Aurora44+
Comment 19 User image percyley 2015-12-01 19:44:45 PST
Why MSN display Firefox 36 support?
Comment 21 User image John Daggett (:jtd) 2015-12-01 20:03:55 PST
(In reply to percyley from comment #19)
> Why MSN display Firefox 36 support?

It was supported initially behind a flag, then only in nightly/developer builds. This bug enables it by default for beta/release builds.

I'll work with the dev-docs folks to clarify this on MDN.
Comment 22 User image David Baron :dbaron: ⌚️UTC-8 2015-12-01 23:54:34 PST
(In reply to John Daggett (:jtd) from comment #8)
> (In reply to David Baron [:dbaron] ⌚UTC-8 from comment #7)
> > One other thought, though (since you made me look at what the test coverage
> > is):  given that the reason for unicode-range to exist is that it allows a
> > way of not bothering to download a font at all based on character ranges, it
> > seems like it's probably worth adding a mochitest to actually test the "lack
> > of download" aspect of it (with a base case that should be downloaded).  It
> > shouldn't be too hard to do in the manner of test_bug517224.html and
> > bug517224.sjs in layout/style/test/ (although you probably want multiple
> > tests, like the somewhat more complicated visited_image_loading test in the
> > same directory).
> 
> Hmmm, that's basically what the slew of tests in
> layout/style/test/test_unicode_range_loading.html are doing? Did you look
> through those? Would you consider those not sufficient? These tests use the
> font loading API rather than .sjs magic. But if needed I'm sure I could come
> up with the kind of test you're thinking about too.

I guess I think it would still be a good idea to write such a test.
Comment 23 User image percyley 2015-12-02 00:36:29 PST
(In reply to John Daggett (:jtd) from comment #21)
> (In reply to percyley from comment #19)
> > Why MSN display Firefox 36 support?
> 
> It was supported initially behind a flag, then only in nightly/developer
> builds. This bug enables it by default for beta/release builds.
> 
> I'll work with the dev-docs folks to clarify this on MDN.

Thanks!
Comment 25 User image Rod 2015-12-02 09:00:18 PST
In preparation for enabling unicode-range for Firefox in Google Fonts I tried out unicode-range on Firefox dev edition 44.0a2 (2015-12-02) and ran into unexpected behavior around src descriptors using local(). 

I have a test that is intended to download two font files (urls ending in ?cyrillic and ?latin-ext). 

If there are no local()'s in src descriptor Firefox downloads three font files: the ones for cyrillic and latin-ext plus one for latin. http://output.jsbin.com/vuragugoza/quiet shows this.

If there are local()'s in the src descriptor Firefox downloads the same three font files and then shortly after that downloads the same three font files again. The Dev Tools Network tab seems to show it downloads the three files, waits a bit, then downloads them again. http://output.jsbin.com/gihumugeya/quiet shows this.

If I make an example where only some src descriptors have local() then only those show as double-downloading.

I had Dev Tools open and Disable Cache (when toolbox is open) enabled when testing.
Comment 26 User image Jonathan Kew (:jfkthame) 2015-12-02 09:22:04 PST
(In reply to Rod from comment #25)
> In preparation for enabling unicode-range for Firefox in Google Fonts I
> tried out unicode-range on Firefox dev edition 44.0a2 (2015-12-02) and ran
> into unexpected behavior around src descriptors using local(). 
> 
> I have a test that is intended to download two font files (urls ending in
> ?cyrillic and ?latin-ext). 

Looking at your example  http://output.jsbin.com/gihumugeya  I think it's expected that the ?latin file will also be downloaded, because neither ?cyrillic nor ?latin-ext lists the U+0020 (space) codepoint in its unicode-range.

> 
> If there are no local()'s in src descriptor Firefox downloads three font
> files: the ones for cyrillic and latin-ext plus one for latin.
> http://output.jsbin.com/vuragugoza/quiet shows this.
> 
> If there are local()'s in the src descriptor Firefox downloads the same
> three font files and then shortly after that downloads the same three font
> files again. The Dev Tools Network tab seems to show it downloads the three
> files, waits a bit, then downloads them again.
> http://output.jsbin.com/gihumugeya/quiet shows this.

I can reproduce this with Developer Edition (44.0a2), but don't see it happening in a Nightly build (45.0a1).

jdaggett, does this sound familiar? A known-and-already-fixed issue?
Comment 27 User image John Daggett (:jtd) 2015-12-02 16:58:14 PST
(In reply to percyley from comment #23)

> > It was supported initially behind a flag, then only in nightly/developer
> > builds. This bug enables it by default for beta/release builds.
> > 
> > I'll work with the dev-docs folks to clarify this on MDN.
> 
> Thanks!

Just updated the unicode-range MDN page to hopefully document current behavior more clearly.
Comment 28 User image John Daggett (:jtd) 2015-12-02 17:04:40 PST
(In reply to Jonathan Kew (:jfkthame) from comment #26)

> > If there are local()'s in the src descriptor Firefox downloads the same
> > three font files and then shortly after that downloads the same three font
> > files again. The Dev Tools Network tab seems to show it downloads the three
> > files, waits a bit, then downloads them again.
> > http://output.jsbin.com/gihumugeya/quiet shows this.
> 
> I can reproduce this with Developer Edition (44.0a2), but don't see it
> happening in a Nightly build (45.0a1).
> 
> jdaggett, does this sound familiar? A known-and-already-fixed issue?

Let me take a look today. If it looks like there's a problem, I'll file a new bug. The repeated downloads may be something related to bug 1188802.
Comment 29 User image Jean-Yves Perrier [:teoli] 2015-12-03 03:40:03 PST
Thanks all for the help in documenting this!
https://developer.mozilla.org/en-US/docs/Web/CSS/%40font-face/unicode-range
and
https://developer.mozilla.org/en-US/Firefox/Releases/44#CSS
Comment 30 User image percyley 2015-12-03 04:01:42 PST
Not Firefox 45?
Comment 31 User image John Daggett (:jtd) 2015-12-03 16:34:03 PST
(In reply to percyley from comment #30)
> Not Firefox 45?

See comment 18, patches have been uplifted to aurora which is what will become Firefox 44.
Comment 32 User image michelesr 2016-02-03 16:27:38 PST
Seems that after release 44 some font families are not matched correctly in Linux platforms.

For example "Liberation Mono" seems to be ignored even if is installed correctly in the system:

$ fc-match 'Liberation Mono'
LiberationMono-Regular.ttf: "Liberation Mono" "Regular"

This didn't happen in previous releases. Is it related to unicode-range?
Comment 33 User image John Daggett (:jtd) 2016-02-03 17:03:09 PST
(In reply to michelesr from comment #32)
> Seems that after release 44 some font families are not matched correctly in
> Linux platforms.
> 
> For example "Liberation Mono" seems to be ignored even if is installed
> correctly in the system:
> 
> $ fc-match 'Liberation Mono'
> LiberationMono-Regular.ttf: "Liberation Mono" "Regular"
> 
> This didn't happen in previous releases. Is it related to unicode-range?

Hmmm, I'm not seeing this with Liberation Mono installed under Ubuntu. What do you see for URL below?

data:text/html,<body style="font-family: Liberation Mono,serif">burp

Serif or monospace font?

If you don't see monospace could you log a new bug?
Comment 34 User image michelesr 2016-02-03 23:47:12 PST
> Hmmm, I'm not seeing this with Liberation Mono installed under Ubuntu. What do you see for URL below?

DejaVu Serif system

Used as: "DejaVu Serif"
Comment 35 User image michelesr 2016-02-03 23:53:48 PST
> Hmmm, I'm not seeing this with Liberation Mono installed under Ubuntu. What do you see for URL below?

Arch Linux here, Linux x86_64

fontconfig version: 2.11.1-2
fontconfig configuration: system provided/untouched 

I noticed "Liberation Mono" stopping working after upgrade from 43 to 44
Comment 36 User image John Daggett (:jtd) 2016-02-04 04:33:59 PST
(In reply to michelesr from comment #34)
> > Hmmm, I'm not seeing this with Liberation Mono installed under Ubuntu. What do you see for URL below?
> 
> DejaVu Serif system
> 
> Used as: "DejaVu Serif"

Logged as bug 1245811

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