Use Ebrima as fallback font for Tifinagh in Windows.

Use Ebrima as fallback font for Tifinagh in Windows.

BUG=569421
Committed: https://crrev.com/85da7a3daf73142f5aedb9e193dc4a59bb0ad0f2
Cr-Commit-Position: refs/heads/master@{#366131}

[Daniel Bratell, 2015-12-15 08:35:32]

bratell@opera.com changed reviewers:
+ eae@chromium.org

[Daniel Bratell, 2015-12-15 08:35:33]

Can you take a look at this? It's a very small change, but I suspect there is
more to it. (Neo) Tifinagh is supported by Microsoft since Windows 7 and I see
other fonts listed here that were also added with Windows 7 (Segoa right?) so I
guess that is ok.

[drott, 2015-12-16 08:52:05]

drott@chromium.org changed reviewers:
+ drott@chromium.org

[drott, 2015-12-16 08:52:06]

https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/text/tifinagh.html (right):

https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/text/tifinagh.html:10: document.write("&#x"
+ charCode.toString(16) + "; (U+" +
Thanks for adding a test, unless there is a specific reason to use
document.write here, I would suggest to use static HTML entities such as &x2d30;
or placing the Tifinagh text directly and adding a <meta charset="utf-8"> in the
header instead of document.write.

[Daniel Bratell, 2015-12-16 09:17:24]

https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/text/tifinagh.html (right):

https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/text/tifinagh.html:10: document.write("&#x"
+ charCode.toString(16) + "; (U+" +
On 2015/12/16 08:52:06, drott wrote:
> Thanks for adding a test, unless there is a specific reason to use
> document.write here, I would suggest to use static HTML entities such as
&x2d30;
> or placing the Tifinagh text directly and adding a <meta charset="utf-8"> in
the
> header instead of document.write.

This is way easier to work with. We can easily check that we have not forgotten
any glyphs, that the two blocks (monospace and default font) contain the same
glyphs. That the labels and the glyphs are referencing the same codepoint.

Since it's pure ASCII it will also not be vulnerable to misunderstandings by
editors or other tools which is particularly important when working with glyphs
that not every tool/environment can render.

It's also possible to quickly reuse this test for other scripts. For instance
https://codereview.chromium.org/1521993008/patch/1/10002 is the same with just
2-3 lines changed. Doing the same thing manually would be much more error prone.

I was going to say that it loses a bit in reviewability, but then I
reconsidered. Reviewing a list of a hundred glyphs, checking that they are all
there, that they use the right code point, the right label, that would be much
harder than reading this script and seeing the start and end of the loop.

(I actually started by modifying an existing testcase that wrote the characters
literally but it was not at all easy and quite timeconsuming so I switched to a
scripted loop instead).

[drott, 2015-12-16 14:57:59]

On 2015/12/16 09:17:24, Daniel Bratell wrote:
>
https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/fast/text/tifinagh.html (right):
> 
>
https://codereview.chromium.org/1525653002/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/fast/text/tifinagh.html:10:
document.write("&#x"
> + charCode.toString(16) + "; (U+" +
> On 2015/12/16 08:52:06, drott wrote:
> > Thanks for adding a test, unless there is a specific reason to use
> > document.write here, I would suggest to use static HTML entities such as
> &x2d30;
> > or placing the Tifinagh text directly and adding a <meta charset="utf-8"> in
> the
> > header instead of document.write.
> 
> This is way easier to work with. We can easily check that we have not
forgotten
> any glyphs, that the two blocks (monospace and default font) contain the same
> glyphs. That the labels and the glyphs are referencing the same codepoint.
> 
> Since it's pure ASCII it will also not be vulnerable to misunderstandings by
> editors or other tools which is particularly important when working with
glyphs
> that not every tool/environment can render.

I'd prefer if you could at the Tifinagh alphabet or a pangram:
LayoutTests/inspector-protocol/layout-fonts/languages-emoji-rare-glyphs.html
(It does not necessarily need to be the full alphabet: As long as the font gets
picked on Windows for USCRIPT_TIFINAGH script and some sample characters, this
would be sufficient as a test in my opinion.)

In this test we keep track of font/language coverage and we can have easily
verifiable baselines for multiple platforms, without resorting to a pixel test.
As you see, this test contains characters from the full unicode range and our
tooling can handle it, so no issues there.

So then, for the Windows expectation we would expect the Ebrima font selected:
LayoutTests/platform/win/inspector-protocol/layout-fonts/languages-emoji-rare-glyphs-expected.txt

We can check the new baseline after the rebaseline bot processed it, or we see
it right away if you could add the updated Windows test expectation to this CL
and let the rebaseline bot just process the other platforms, where the fallback
mechanism likely chooses a different font.

[Daniel Bratell, 2015-12-17 09:23:53]

drott, please take a new look.

I've added a new test using the inspector framework. I've kept the glyph test
though because there are so much that could potentially go wrong after the font
selection and we would most likely not have any way of knowing until after a
full release.

I also changed it from document.write to innerHTML though I believe
document.write being the more readable solution. I know document.write sucks in
many ways (I've probably spent hundreds of hours on document.write related
issues and implementations) but it's useful in cases like this.

[drott, 2015-12-18 11:44:53]

Hi Daniel,

On 2015/12/17 09:23:53, Daniel Bratell wrote:
> drott, please take a new look.

I really appreciate it, thanks for taking the the time to update the test. Test
and expectations look good to me for the new test, with a license nit on the
source of the sample text.

> I've added a new test using the inspector framework. I've kept the glyph test
> though because there are so much that could potentially go wrong after the
font
> selection and we would most likely not have any way of knowing until after a
> full release.

In the past, we've had issues with test flakiness or extra work for rebaselining
from tests that test for glyph rendering of rare glyphs due to randomness in
fallback behavior. For example in
fast/text/midword-break-before-surrogate-pair.html ) The test results sometimes
flake due to differences between bots, especially on Linux were the bot fleet
has different versions of Ubuntu (and differing glyphs for the .notdef glyph) or
on Mac where the font configuration differs between OS versions. 

I agree it would be useful to also test whether the glyphs show up on screen,
but to make the testing more robust, I'd prefer if this would be backed by a web
font and test the fallback mechanism separately - or we can leave the glyph
table test out for now - what's your opinion?





> 
> I also changed it from document.write to innerHTML though I believe
> document.write being the more readable solution. I know document.write sucks
in
> many ways (I've probably spent hundreds of hours on document.write related
> issues and implementations) but it's useful in cases like this.

https://codereview.chromium.org/1525653002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/tifinagh.html
(right):

https://codereview.chromium.org/1525653002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/tifinagh.html:12:
<div id="tifinagh_wikipedia_text"><!-- First part of
https://incubator.wikimedia.org/wiki/Wp/shi/Igudar/ⵜⵉⴼⵉⵏⴰⵖ --> ⴰⴳⴰⴷⵉⵔ ⵉⵖ ⴳⴳⵓⵜⵏ
ⴰⵔ ⴰⵙⵏ ⵏⵜⵜⵉⵏⵉ ⵉⴳⵓⴷⴰⵔ ⴳⴰⵏ ⵢⴰⵜ ⵍⴱⴰⵏⴽ ⵜⴰⵎⵣⵡⴰⵔⵓⵜ ⵏⵉⵖ ⵜⴰⴱⵍⴷⵉⵜ ⴰⵔ ⴳⵉⵙ ⵜⵜⴳⴳⴰⵏ ⵎⵉⴷⴷⵏ
ⵜⵓⵎⵥⵉⵏ ⵏⵖ ⵉⵔⴷⵏ ⵏⵖ ⵉⵇⵇⴰⵔⵉⴷⵏ ⴰⴳⴰⴷⵉⵔ ⵉⴳⴰ ⵍⴱⴰⵏⴽ ⵍⵍⵉ ⴷ ⴽⵓⵍⵍⵓ ⵉⵣⵡⴰⵔⵏ ⵖ ⴷⴷⵓⵏⵉⵜ ⵍⵍⴰⵏ
ⵜⵓⴳⵜ ⵏ ⵉⴳⵓⴷⴰⵔ ⵖ ⵜⵙⴳⴰ ⵏ ⵙⵓⵙ
Could you perhaps find a non-copyrighted sample text (perhaps there is some
publicly available literature), or just use the alphabet? I don't think we can
include content from Wikipedia into our tests.

[Daniel Bratell, 2015-12-18 15:15:42]

drott, this might be better!

https://codereview.chromium.org/1525653002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/tifinagh.html
(right):

https://codereview.chromium.org/1525653002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-protocol/layout-fonts/tifinagh.html:12:
<div id="tifinagh_wikipedia_text"><!-- First part of
https://incubator.wikimedia.org/wiki/Wp/shi/Igudar/ⵜⵉⴼⵉⵏⴰⵖ --> ⴰⴳⴰⴷⵉⵔ ⵉⵖ ⴳⴳⵓⵜⵏ
ⴰⵔ ⴰⵙⵏ ⵏⵜⵜⵉⵏⵉ ⵉⴳⵓⴷⴰⵔ ⴳⴰⵏ ⵢⴰⵜ ⵍⴱⴰⵏⴽ ⵜⴰⵎⵣⵡⴰⵔⵓⵜ ⵏⵉⵖ ⵜⴰⴱⵍⴷⵉⵜ ⴰⵔ ⴳⵉⵙ ⵜⵜⴳⴳⴰⵏ ⵎⵉⴷⴷⵏ
ⵜⵓⵎⵥⵉⵏ ⵏⵖ ⵉⵔⴷⵏ ⵏⵖ ⵉⵇⵇⴰⵔⵉⴷⵏ ⴰⴳⴰⴷⵉⵔ ⵉⴳⴰ ⵍⴱⴰⵏⴽ ⵍⵍⵉ ⴷ ⴽⵓⵍⵍⵓ ⵉⵣⵡⴰⵔⵏ ⵖ ⴷⴷⵓⵏⵉⵜ ⵍⵍⴰⵏ
ⵜⵓⴳⵜ ⵏ ⵉⴳⵓⴷⴰⵔ ⵖ ⵜⵙⴳⴰ ⵏ ⵙⵓⵙ
On 2015/12/18 11:44:52, drott wrote:
> Could you perhaps find a non-copyrighted sample text (perhaps there is some
> publicly available literature), or just use the alphabet? I don't think we can
> include content from Wikipedia into our tests.

Done.

[drott, 2015-12-18 16:03:36]

LGTM, thanks!

[drott, 2015-12-18 16:04:00]

The CQ bit was checked by drott@chromium.org

[commit-bot: I haz the power, 2015-12-18 16:04:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1525653002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1525653002/100001

[commit-bot: I haz the power, 2015-12-18 19:08:28]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-18 19:09:14]

Description was changed from

==========
Use Ebrima as fallback font for Tifinagh in Windows.

BUG=569421
==========

to

==========
Use Ebrima as fallback font for Tifinagh in Windows.

BUG=569421

Committed: https://crrev.com/85da7a3daf73142f5aedb9e193dc4a59bb0ad0f2
Cr-Commit-Position: refs/heads/master@{#366131}
==========

[commit-bot: I haz the power, 2015-12-18 19:09:14]

Patchset 6 (id:??) landed as
https://crrev.com/85da7a3daf73142f5aedb9e193dc4a59bb0ad0f2
Cr-Commit-Position: refs/heads/master@{#366131}