Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use font loader api #5366

Merged
merged 2 commits into from
Sep 30, 2014
Merged

Use font loader api #5366

merged 2 commits into from
Sep 30, 2014

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Sep 30, 2014

See #5365.

Tested in Chromium 37 (and verified that it does not break Firefox 32 or Chromium 34).

This name clashes with the FontFace constructor from the Font Loading CSS module:
http://dev.w3.org/csswg/css-font-loading/#font-face-constructor

isFontLoadingAPISupported: !isWorker && !!document.fonts,

addNativeFontFace: function fontLoader__addNativeFontFace(nativeFontFace) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: fontLoader__addNativeFontFace (and FontFaceObject__createNativeFontFace below), can you use one '_'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@yurydelendik
Copy link
Contributor

That was fast... I think it looks good.

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/1671e686e708d3e/output.txt

@Rob--W Rob--W force-pushed the use-font-loader-api branch from 6f0092c to 169909a Compare September 30, 2014 20:17
@Rob--W Rob--W force-pushed the use-font-loader-api branch from 169909a to d0845df Compare September 30, 2014 20:18
@yurydelendik
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/df29877e4d642af/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/334f65f03d33a04/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/df29877e4d642af/output.txt

Total script time: 19.47 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/334f65f03d33a04/output.txt

Total script time: 22.43 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@yurydelendik
Copy link
Contributor

now using chrome beta

/botio-linux test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/f6583cbd1eb57f4/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/f6583cbd1eb57f4/output.txt

Total script time: 22.86 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/f6583cbd1eb57f4/reftest-analyzer.html#web=eq.log

@CodingFabian
Copy link
Contributor

could this also affect the print issue we were chasing?

@yurydelendik
Copy link
Contributor

@Rob--W looks good, thank you

/botio makeref

could this also affect the print issue we were chasing?

@CodingFabian that's implemented for Chrome for now, not sure if that will fix the printing issues atm

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/8bd3fb750e825af/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/c59abeb015a2a9b/output.txt

yurydelendik added a commit that referenced this pull request Sep 30, 2014
@yurydelendik yurydelendik merged commit 1858fbf into mozilla:master Sep 30, 2014
@CodingFabian
Copy link
Contributor

@yurydelendik interestingly this does have a effect. the font is right now ok after cleanup, the page size however is broken.

@yurydelendik
Copy link
Contributor

@CodingFabian could you be more specific with STR and browser version(s)?

@yurydelendik
Copy link
Contributor

@CodingFabian can it be a regression from other PRs?

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/8bd3fb750e825af/output.txt

Total script time: 21.35 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/c59abeb015a2a9b/output.txt

Total script time: 22.45 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@CodingFabian
Copy link
Contributor

i used the pdf mentioned in #5006. The font comes out fine. I have yet to bisect it. Right now at javaone conference, and wanted to give it a quick check, might bisect it tonight from hotel.
STR: FF 32.0.3 on Mac. Open PDF and hit print. Select Save as PDF

@yurydelendik
Copy link
Contributor

@CodingFabian looks like a10fde1 is first bad commit

@CodingFabian
Copy link
Contributor

/cc @Snuffleupagus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants