Noissue - Update font-family to Source Sans Pro on abp.org

Noissue - Update font-family to Source Sans Pro on abp.org

Branch: index_page https://hg.adblockplus.org/web.adblockplus.org/shortlog/index_page

[ire, 2018-02-09 12:34:08]

@Manvel please take a look at this when you can

[juliandoucette, 2018-02-13 18:32:06]

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:6: src:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.eot");
I don't think this isn't necessary unless we want to load this font in IE
compatibility modes (which I don't think we do).

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:7: src:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.eot?#iefix")
format("embedded-opentype"),
I don't think that this isn't necessary unless we want to load this font on IE
6-8 (which I don't think we do).

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:12:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.ttf") format("truetype"),
I don't think this is necessary unless we want to load this font on old versions
of mobile browsers (which I don't think we do).

Detail: This would be necessary to support Android >= 4.0.3 in our
browserlistrc. However, older versions of Android also imply older (less
capable) phones and slower network connection?

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:13:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.svg#SourceSansPro")
format("svg");
I don't think that this is necessary unless we want to support old versions of
iOS (which I don't think we do).

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:33: font-size: 16px;
I think we can rely on website-defaults to set (or intentionally not set) the
body font-size and weight.

[ire, 2018-02-14 08:43:42]

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:6: src:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.eot");
On 2018/02/13 18:32:06, juliandoucette wrote:
> I don't think this isn't necessary unless we want to load this font in IE
> compatibility modes (which I don't think we do).

Done.

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:7: src:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.eot?#iefix")
format("embedded-opentype"),
On 2018/02/13 18:32:05, juliandoucette wrote:
> I don't think that this isn't necessary unless we want to load this font on IE
> 6-8 (which I don't think we do).

Done.

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:12:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.ttf") format("truetype"),
On 2018/02/13 18:32:06, juliandoucette wrote:
> I don't think this is necessary unless we want to load this font on old
versions
> of mobile browsers (which I don't think we do).
> 
> Detail: This would be necessary to support Android >= 4.0.3 in our
> browserlistrc. However, older versions of Android also imply older (less
> capable) phones and slower network connection?

Done.

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:13:
url("../fonts/Source-Sans-Pro-400/Source-Sans-Pro-400.svg#SourceSansPro")
format("svg");
On 2018/02/13 18:32:06, juliandoucette wrote:
> I don't think that this is necessary unless we want to support old versions of
> iOS (which I don't think we do).

Done.

https://codereview.adblockplus.org/29693561/diff/29693562/static/css/main.css...
static/css/main.css:33: font-size: 16px;
On 2018/02/13 18:32:06, juliandoucette wrote:
> I think we can rely on website-defaults to set (or intentionally not set) the
> body font-size and weight.

Done.

[saroyanm, 2018-02-14 16:49:30]

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:1: @font-face
Question: How much effort it would be to include Fonts implementation in the
Websites default ?
Is there a such plan ?

(I assume we use same fonts across all websites)

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro";
Suggestion/Note: Doesn't necessarily need to be included in the current
review/release.

I assume current fonts doesn't cover all supported languages ?
For example I assume they do not contain ex. cyrillic, greek characters, but
rather only Latin.

With: https://codereview.adblockplus.org/29654555/ we are trying to fix that for
the UI, but I think it's also relevant to the websites.

[ire, 2018-02-15 17:21:47]

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:1: @font-face
On 2018/02/14 16:49:29, saroyanm wrote:
> Question: How much effort it would be to include Fonts implementation in the
> Websites default ?
> Is there a such plan ?
> 
> (I assume we use same fonts across all websites)

I think we would ideally want to have this font in website-defaults and share it
across websites, but it loading a file directly from website-defaults would
require that we add a build step (since the website-defaults repository would be
managed via npm). 

We don't currently have website-defaults in this project (currently Julian has
been manually adding the CSS to defaults.css), so I don't think doing this would
work until we decide to add it.

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro";
On 2018/02/14 16:49:30, saroyanm wrote:
> Suggestion/Note: Doesn't necessarily need to be included in the current
> review/release.
> 
> I assume current fonts doesn't cover all supported languages ?
> For example I assume they do not contain ex. cyrillic, greek characters, but
> rather only Latin.
> 
> With: https://codereview.adblockplus.org/29654555/ we are trying to fix that
for
> the UI, but I think it's also relevant to the websites.

From my tests (viewing the homepage in all the different languages) there didn't
seem to be any issue with any of the languages. But from looking though the
review you mentioned, it seems like you did come across issues in certain
languages. If that is the case, we could follow your methodology here of loading
different fonts for the different subsets.

I would prefer to start with just this and have a tester see if there is an
issue to begin with though, just to make sure it's actually necessary in our
case. What do you think?

[juliandoucette, 2018-02-19 11:32:49]

Thanks Ire!

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:1: @font-face
On 2018/02/15 17:21:47, ire wrote:
> On 2018/02/14 16:49:29, saroyanm wrote:
> > Question: How much effort it would be to include Fonts implementation in the
> > Websites default ?
> > Is there a such plan ?
> > 
> > (I assume we use same fonts across all websites)
> 
> I think we would ideally want to have this font in website-defaults and share
it
> across websites, but it loading a file directly from website-defaults would
> require that we add a build step (since the website-defaults repository would
be
> managed via npm). 
> 
> We don't currently have website-defaults in this project (currently Julian has
> been manually adding the CSS to defaults.css), so I don't think doing this
would
> work until we decide to add it. 

Acknowledged.

https://codereview.adblockplus.org/29693561/diff/29697558/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro";
On 2018/02/15 17:21:46, ire wrote:
> I would prefer to start with just this and have a tester see if there is an
> issue to begin with though, just to make sure it's actually necessary in our
> case. What do you think?

I think that we can start with Latin only in main.css and include variants
as-needed via template (separately, assuming that including this latin font will
not have a negative impact on existing translations). We should ask Tamara
though. Will you do that Ire?

https://codereview.adblockplus.org/29693561/diff/29697558/static/fonts/Source...
File static/fonts/Source-Sans-Pro-600/LICENSE.txt (right):

https://codereview.adblockplus.org/29693561/diff/29697558/static/fonts/Source...
static/fonts/Source-Sans-Pro-600/LICENSE.txt:1: Copyright 2010, 2012, 2014 Adobe
Systems Incorporated (http://www.adobe.com/), with Reserved Font Name 'Source'.
All Rights Reserved. Source is a trademark of Adobe Systems Incorporated in the
United States and/or other countries.
This license text seems to be a duplicate of the one included in 400. Is there a
good reason to put these two font weights in separate folders and duplicate the
license text?

[ire, 2018-02-26 19:02:54]

@Manvel : Any update on your findings from your other review? I'm waiting on
that to know which variants we would need to include here.

https://codereview.adblockplus.org/29693561/diff/29697558/static/fonts/Source...
File static/fonts/Source-Sans-Pro-600/LICENSE.txt (right):

https://codereview.adblockplus.org/29693561/diff/29697558/static/fonts/Source...
static/fonts/Source-Sans-Pro-600/LICENSE.txt:1: Copyright 2010, 2012, 2014 Adobe
Systems Incorporated (http://www.adobe.com/), with Reserved Font Name 'Source'.
All Rights Reserved. Source is a trademark of Adobe Systems Incorporated in the
United States and/or other countries.
On 2018/02/19 11:32:49, juliandoucette wrote:
> This license text seems to be a duplicate of the one included in 400. Is there
a
> good reason to put these two font weights in separate folders and duplicate
the
> license text?

I think there was a reason I originally did it in the help center, but I can't
remember right now and I agree it probably doesn't make sense. So I will change
it.

[saroyanm, 2018-02-27 10:57:26]

On 2018/02/26 19:02:54, ire wrote:
> @Manvel : Any update on your findings from your other review? I'm waiting on
> that to know which variants we would need to include here.
Suggestions:
I think we can include all variants introduce in the review
(https://codereview.adblockplus.org/29654555/), the only thing is that I
couldn't find woff version of fonts and there are still languages that I
couldn't find bundles for and I'm not sure if they are supported ->
https://issues.adblockplus.org/ticket/6407

For the woff version I think we can include the one for the current review,
which shouldn't introduce a regression for the old browsers, but we will support
more lagnuages for modern browsers.

Also noticed that we do not have abp.org translated Vietnamese, but as we are
planing to introduce font bundles into the websites-default, I think we can add
Vietnamese as well for now.

[ire, 2018-02-28 09:14:19]

On 2018/02/27 10:57:26, saroyanm wrote:
> On 2018/02/26 19:02:54, ire wrote:
> > @Manvel : Any update on your findings from your other review? I'm waiting on
> > that to know which variants we would need to include here.
> Suggestions:
> I think we can include all variants introduce in the review
> (https://codereview.adblockplus.org/29654555/), the only thing is that I
> couldn't find woff version of fonts and there are still languages that I
> couldn't find bundles for and I'm not sure if they are supported ->
> https://issues.adblockplus.org/ticket/6407
> 
> For the woff version I think we can include the one for the current review,
> which shouldn't introduce a regression for the old browsers, but we will
support
> more lagnuages for modern browsers.
> 
> Also noticed that we do not have http://abp.org translated Vietnamese, but as
we are
> planing to introduce font bundles into the websites-default, I think we can
add
> Vietnamese as well for now.

Thanks Manvel. Could you help me answer the following questions about your
implementation:

1. Why did you use the `local("Ø")` method to disallow using a local "Source
Sans Pro" font?
2. In your implementation, you didn't also use the regular font files (i.e. not
variant-specific). Is there a reason for that since you are suggesting I do that
here?
3. Where did you get the font files? You are using the font weights 300, 400,
and 700. Whereas here we only use 400 and 600 (although we may want to change
this to be consistent with the UI implementation)

Thank you

[saroyanm, 2018-02-28 09:52:05]

On 2018/02/28 09:14:19, ire wrote:
> On 2018/02/27 10:57:26, saroyanm wrote:
> > On 2018/02/26 19:02:54, ire wrote:
> > > @Manvel : Any update on your findings from your other review? I'm waiting
on
> > > that to know which variants we would need to include here.
> > Suggestions:
> > I think we can include all variants introduce in the review
> > (https://codereview.adblockplus.org/29654555/), the only thing is that I
> > couldn't find woff version of fonts and there are still languages that I
> > couldn't find bundles for and I'm not sure if they are supported ->
> > https://issues.adblockplus.org/ticket/6407
> > 
> > For the woff version I think we can include the one for the current review,
> > which shouldn't introduce a regression for the old browsers, but we will
> support
> > more lagnuages for modern browsers.
> > 
> > Also noticed that we do not have http://abp.org translated Vietnamese, but
as
> we are
> > planing to introduce font bundles into the websites-default, I think we can
> add
> > Vietnamese as well for now.
> 
> Thanks Manvel. Could you help me answer the following questions about your
> implementation:
> 
> 1. Why did you use the `local("Ø")` method to disallow using a local "Source
> Sans Pro" font?
I think we were using it in order to be sure that the font is used by webpage is
the one we are providing rather than relying on user's local font, but I'm not
completely sure if that's necessary in Websites (We don't have issue of
downloading fonts in UI).
> 2. In your implementation, you didn't also use the regular font files (i.e.
not
> variant-specific). Is there a reason for that since you are suggesting I do
that
> here?
400 are the regular one I guess, at least that's what is being provided by
Google API when the size is not specified.
> 3. Where did you get the font files? You are using the font weights 300, 400,
> and 700. Whereas here we only use 400 and 600 (although we may want to change
> this to be consistent with the UI implementation)
I'm using google's fonts API
https://fonts.googleapis.com/css?family=Source+Sans+Pro:300&subset=cyrillic,c...
Yes, or maybe we will change to be consistent with the websites. I assume Jeen
can help us here.
> Thank you

[ire, 2018-03-01 14:00:16]

Thanks Manvel! New patch set uploaded.

A few comments:

1. Jeen suggested we use the same font weights as the abdlockplusui, so I've
added those font weights here
2. We only have access to variant-specific woff2 files, which still isn't
supported in IE (https://caniuse.com/#search=woff2). I'm not sure how to proceed
here, it appears we will have to choose between using the old font files, which
have the wider-supported woff files, or use these variant-specific files that
are only available in woff2. I don't know if it is possible to use both without
essentially loading multiple of the same thing.

[saroyanm, 2018-03-13 12:58:19]

Sorry, for late reply.
LGTM

https://codereview.adblockplus.org/29693561/diff/29712605/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29712605/static/css/fonts.cs...
static/css/fonts.css:1:
/******************************************************************************
Note: This file is missing the header, but considering fact that other CSS files
are missing the header as well, we can tackle addition of header separately.

https://codereview.adblockplus.org/29693561/diff/29712605/static/fonts/Source...
File static/fonts/Source-Sans-Pro/LICENSE.txt (right):

https://codereview.adblockplus.org/29693561/diff/29712605/static/fonts/Source...
static/fonts/Source-Sans-Pro/LICENSE.txt:1: Copyright 2010, 2012 Adobe Systems
Incorporated (http://www.adobe.com/), with Reserved Font Name 'Source'. All
Rights Reserved. Source is a trademark of Adobe Systems Incorporated in the
United States and/or other countries.
Deatail: Missing 2014 ->
https://github.com/adobe-fonts/source-sans-pro/blob/release/LICENSE.txt

[ire, 2018-03-13 13:50:51]

Thanks Manvel! I've updated the licence.

Julian, any comments on this before I commit&push?

https://codereview.adblockplus.org/29693561/diff/29712605/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29712605/static/css/fonts.cs...
static/css/fonts.css:1:
/******************************************************************************
On 2018/03/13 12:58:18, saroyanm wrote:
> Note: This file is missing the header, but considering fact that other CSS
files
> are missing the header as well, we can tackle addition of header separately.

Ack. Agreed.

https://codereview.adblockplus.org/29693561/diff/29712605/static/fonts/Source...
File static/fonts/Source-Sans-Pro/LICENSE.txt (right):

https://codereview.adblockplus.org/29693561/diff/29712605/static/fonts/Source...
static/fonts/Source-Sans-Pro/LICENSE.txt:1: Copyright 2010, 2012 Adobe Systems
Incorporated (http://www.adobe.com/), with Reserved Font Name 'Source'. All
Rights Reserved. Source is a trademark of Adobe Systems Incorporated in the
United States and/or other countries.
On 2018/03/13 12:58:18, saroyanm wrote:
> Deatail: Missing 2014 ->
> https://github.com/adobe-fonts/source-sans-pro/blob/release/LICENSE.txt

Done.

[juliandoucette, 2018-03-13 14:41:13]

On 2018/03/13 13:50:51, ire wrote:
> Thanks Manvel! I've updated the licence.
> 
> Julian, any comments on this before I commit&push?

Yes. I may not get to this today unless you twist my arm though.

[juliandoucette, 2018-03-14 17:08:39]

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmpl
File includes/styles.tmpl (right):

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmp...
includes/styles.tmpl:2: <link rel="stylesheet" href="/css/fonts.css">
Why a separate file?

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.cs...
static/css/fonts.css:5: /* cyrillic-ext
Can't we use a jinja template to place these styles in the <head> in languages
that use them?

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro", Arial, sans-serif;
My browsers are rendering Arial when I run the testserver. Is that expected?

[saroyanm, 2018-03-14 17:56:30]

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmpl
File includes/styles.tmpl (right):

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmp...
includes/styles.tmpl:2: <link rel="stylesheet" href="/css/fonts.css">
On 2018/03/14 17:08:38, juliandoucette wrote:
> Why a separate file?

Ideally this will go to the websites default to import it for multilingual
websites.

Note: We use this file in UI separately in order to load in multiple pages, but
I don't think you have this need in current repo.
Not necessarily this needs to be a separate file here.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.cs...
static/css/fonts.css:5: /* cyrillic-ext
On 2018/03/14 17:08:38, juliandoucette wrote:
> Can't we use a jinja template to place these styles in the <head> in languages
> that use them?

That's an interesting approach, but you can't determine unicode-ranges with the
Jinja and probably it's simpler to rely on the browser to do that tedious task
for us.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro", Arial, sans-serif;
On 2018/03/14 17:08:38, juliandoucette wrote:
> My browsers are rendering Arial when I run the testserver. Is that expected?

No, until you are inspecting element with the characters that are not supported
by the provided font files and unicode-ranges, for example Chinese.

Note: I haven't tested myself yet.

[ire, 2018-03-14 18:10:34]

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmpl
File includes/styles.tmpl (right):

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmp...
includes/styles.tmpl:2: <link rel="stylesheet" href="/css/fonts.css">
On 2018/03/14 17:08:38, juliandoucette wrote:
> Why a separate file?

I did it this way because it just seemed cleaner to separate it. Same the
defaults css is separate (I think, anyway)?

Also, when we implement the build step here everything will be in one file.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.cs...
static/css/fonts.css:5: /* cyrillic-ext
On 2018/03/14 17:08:38, juliandoucette wrote:
> Can't we use a jinja template to place these styles in the <head> in languages
> that use them?

We don't need to. The fonts should only be loaded if the unicode-range matches
characters used on the page.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro", Arial, sans-serif;
On 2018/03/14 17:08:38, juliandoucette wrote:
> My browsers are rendering Arial when I run the testserver. Is that expected?

I just fixed this. I missed a comma somewhere (*facepalm*)

[juliandoucette, 2018-03-14 18:18:03]

LGTM + superNIT

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmpl
File includes/styles.tmpl (right):

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmp...
includes/styles.tmpl:2: <link rel="stylesheet" href="/css/fonts.css">
On 2018/03/14 18:10:31, ire wrote:
> On 2018/03/14 17:08:38, juliandoucette wrote:
> > Why a separate file?
> 
> I did it this way because it just seemed cleaner to separate it. Same the
> defaults css is separate (I think, anyway)?

Yes and no. I separated default.css so that I could update default.css when
website-defaults updates without seeming like I'd changed main.css.

> Also, when we implement the build step here everything will be in one file. 

Agreed.

---

I'd prefer to update website-defaults and then update defaults.css in
web.adblockplus.org in order to include this CSS in defaults.css. But that's a
super-NIT preference. It only matters in the rare case that a user who has a bad
connection is having a hard time downloading this small file (which will rarely
if ever happen).

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.css
File static/css/fonts.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.cs...
static/css/fonts.css:5: /* cyrillic-ext
On 2018/03/14 17:56:30, saroyanm wrote:
> On 2018/03/14 17:08:38, juliandoucette wrote:
> > Can't we use a jinja template to place these styles in the <head> in
languages
> > that use them?
> 
> That's an interesting approach, but you can't determine unicode-ranges with
the
> Jinja and probably it's simpler to rely on the browser to do that tedious task
> for us.

Acknowledged.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/fonts.cs...
static/css/fonts.css:5: /* cyrillic-ext
On 2018/03/14 18:10:31, ire wrote:
> On 2018/03/14 17:08:38, juliandoucette wrote:
> > Can't we use a jinja template to place these styles in the <head> in
languages
> > that use them?
> 
> We don't need to. The fonts should only be loaded if the unicode-range matches
> characters used on the page.

Acknowledged.

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29693561/diff/29721646/static/css/main.css...
static/css/main.css:3: font-family: "Source Sans Pro", Arial, sans-serif;
On 2018/03/14 18:10:33, ire wrote:
> On 2018/03/14 17:08:38, juliandoucette wrote:
> > My browsers are rendering Arial when I run the testserver. Is that expected?
> 
> I just fixed this. I missed a comma somewhere (*facepalm*)

Glad you spotted it!

I couldn't figure it out :D

[ire, 2018-03-15 08:30:49]

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmpl
File includes/styles.tmpl (right):

https://codereview.adblockplus.org/29693561/diff/29721646/includes/styles.tmp...
includes/styles.tmpl:2: <link rel="stylesheet" href="/css/fonts.css">
On 2018/03/14 18:18:00, juliandoucette wrote: 
> I'd prefer to update website-defaults and then update defaults.css in
> http://web.adblockplus.org in order to include this CSS in defaults.css. But
that's a
> super-NIT preference. It only matters in the rare case that a user who has a
bad
> connection is having a hard time downloading this small file (which will
rarely
> if ever happen).

I agree, but adding it to website-defaults may not be so straightforward since
we will probably have to make the path to the font files configurable. 

I will push this as it is for now and open a separate review for that