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

safari 10.1 - bad fonts and italics #8231

Closed
paolocaminiti opened this issue Apr 3, 2017 · 22 comments
Closed

safari 10.1 - bad fonts and italics #8231

paolocaminiti opened this issue Apr 3, 2017 · 22 comments

Comments

@paolocaminiti
Copy link

Link to PDF file (or attach file here):
https://www.ncbi.nlm.nih.gov/pmc/articles/PMC3847663/pdf/fnins-07-00231.pdf

Configuration:

  • Web browser and its version:
    Safari 10.1 (12603.1.30.0.34)

  • Operating system and its version:
    OSX 10.12.4 (16E195)

  • PDF.js version:
    latest demo from github

  • Is an extension:
    no

Steps to reproduce the problem:

  1. open pdf
  2. check font family and style of start of document
  3. the bug is not present on previous versions of Safari

What is the expected behavior? (add screenshot)
Font should load and it's italic style display.
screen shot 2017-04-03 at 18 04 04

What went wrong? (add screenshot)
Font looks like a fall-back font, anyway no italic variant rendered.
screen shot 2017-04-03 at 18 10 03

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

@paolocaminiti
Copy link
Author

paolocaminiti commented Apr 5, 2017

This issue appeared on and only applies to Safari 10.1, which if I don't go wrong was released end of March 2017.

@rianmcguire
Copy link

I've just spent a couple of hours trying to understand this - we've been experiencing the same issue with a different PDF.

Safari 10 introduced support for the font loading API, so there have been some changes to the browser in this area.

FontLoader#addNativeFontFace gets called, the font is added, the loaded promise from the FontFace resolves successfully, but the font just doesn't render when drawn on the canvas. This happens reliably. Weirdly it doesn't seem to affect all fonts in the document.

I've also noticed that, sometimes, when the document re-renders it will magically start working. This happens so infrequently that it makes me doubt my sanity. It seems to be more likely to happen if you leave the window for a minute, and then trigger a render by zooming.

Forcing FontLoader.isFontLoadingAPISupported to false doesn't help either - the prepareFontLoadEvent hack also thinks the font is loaded.

@evadne
Copy link
Contributor

evadne commented Apr 11, 2017

Looks like a known issue.
https://github.com/bramstein/fontfaceobserver/pull/86/files
https://bugs.webkit.org/show_bug.cgi?id=165037
https://bugs.webkit.org/show_bug.cgi?id=164902

Unfortunately not easily fixed by swapping nativeFontFace.loaded with nativeFontFace.load() in FontLoader.prototype.bind

Thought: if you put a known symbol in the generated font, you could repeatedly draw that to a canvas and measure the pixels to see if the font has been loaded 😬

@Tilmn
Copy link

Tilmn commented May 31, 2017

I experience the same problem in Chrome, when PDF.js is used embedded in Seafile.com

bildschirmfoto 2017-05-31 um 10 29 13

The dropped fonts happen very randomly. If the document was loaded before, it does not happen at all. It seems to happen more with more complex PDFs.

Hope that does help a bit. I hope we can sort this out, it‘s a real problem for us.

@yurydelendik
Copy link
Contributor

Hope that does help a bit. I hope we can sort this out, it‘s a real problem for us.

@Tilmn I don't think it's related to the Safari problem discussed here and it can be moved to the separate issue (but you need to provide all requested details). Also we are not familiar with Seafile.com and screenshot does not show interface of PDF.js. Please contact vendors of web site for support -- you need to provide enough details for us to reproduce the issue.

@wanmigs
Copy link

wanmigs commented Jun 5, 2017

i solved the problem by converting the pdf to lower version. i've tested the pdf with version 1.7 and the issues occur but when i converted the pdf to version 1.4 with ghostscript the pdf renders well.

@MSchip
Copy link

MSchip commented Jul 6, 2017

If anybody has any other temporary solutions please share. This has become a rather large issue for us.

@franky-continu
Copy link

@wanmigs sorry to ping you, would you mind dropping a few lines on how did you do the version change with ghostscript?

I wonder if this would do the trick as well?

https://www.techwalla.com/articles/how-to-convert-pdf-files-to-an-older-format

@wanmigs
Copy link

wanmigs commented Jul 11, 2017

@franky-continu make sure to have ghostscipt installed on your server.

$new_pdf = 'path/new.pdf'; // path where to output the new pdf
$src_pdf = 'path/src/pdf';
shell_exec( "gs -sDEVICE=pdfwrite -dCompatibilityLevel=1.4 -dNOPAUSE -dQUIET -dBATCH -sOutputFile={$new_pdf} {$src_pdf}");

Hope that does help a bit.

@rijk
Copy link

rijk commented Aug 31, 2017

It appears they have fixed this issue at Box. Any chance this fix can be pulled into this repo as well?

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Aug 31, 2017

It appears they have fixed this issue at Box. Any chance this fix can be pulled into this repo as well?

I'm not sure if box/box-content-preview#332 is entirely relevant here, since that's a downstream consumer of the PDF.js library. (And what they did is simply to apply a work-around, not a proper fix, one that could very well end up breaking more things than it solves.)

There's currently no solution to this on the PDF.js side. However, the fact that this appears to be limited to a particular version of just one browser strongly suggests that it's a font handling bug in Safari (rather than in PDF.js).

@rijk
Copy link

rijk commented Aug 31, 2017

All right, sorry I'm not familiar with the innards of either library. Was just hopeful. If there is no other fix than waiting for a Safari update, is there at least a workaround other than converting the files? I don't want to be dramatic but we are actually losing customers over this (as our customer base is agencies, who are predominantly on Safari).

@ajnyga
Copy link

ajnyga commented Aug 31, 2017

Open Journal Systems uses PDF.js library so I have been following this thread. Regarding the statement that this is a Safari bug, I actually know that Apple disagrees.

One journal editor has been in contact with Apple regarding the issue and they think that the issue is with PDF.js. So waiting for a fix in the Safari end could last forever.

@danlester
Copy link

I'm not 100% sure if we're talking about the same "Safari bugs" but I've seen a great improvement in Safari's rendering under the Safari Technology Preview, so hoping those fixes get rolled out to general Safari soon:

https://developer.apple.com/safari/technology-preview/

@rijk
Copy link

rijk commented Aug 31, 2017

I think it definitely should be fixable. Just now, I uploaded this file to Dropbox, and previewed it. The first time, everything went perfect, so I assumed they were using a different engine. Then I refreshed, and saw the exact issues I reported today. Now, every time I open the file, there is a flash of it looking perfect while it loads (this might be an image preview but I don't think so, as I was definitely able to interact with it the first time — even select the text), and then when it finishes loading all the text suddenly shifts. You should be able to reproduce it in Safari with this link.

Edit: here is a screen recording demonstrating the issue: https://www.youtube.com/watch?v=WqIkpjHMSVo

So this establishes Safari is capable of displaying it properly. There must be something that's triggered in the library, that makes Safari go bad. If we find out what it is, I bet we can fix it!

PS: @danlester I can confirm the Safari Technology Preview does not have this problem! 👍 Unfortunately this does not help us now..

@yurydelendik
Copy link
Contributor

There must be something that's triggered in the library, that makes Safari go bad. If we find out what it is, I bet we can fix it!

I can help a bit: modern Safari (as any other modern browsers) use Font Loading API (see https://drafts.csswg.org/css-font-loading/). PDF.js waits on Promise coming from browser to know when fonts are loaded (see code around fontLoadPromises variable).

@rijk can you override my analysis and confirm that PDF.js at fault here and PDF.js is doing something different for Safari or not using the API properly?

@rijk
Copy link

rijk commented Aug 31, 2017

@yurydelendik, not sure what you mean with "can you override my analysis", but maybe my screen recording helps?

I cannot tell you if the bug is in Safari or PDF.js. But the combination seems to break it; triggering the font loading just completely breaks the rendered page. And that's exactly what the Box fix seems to consist of as well: disabling the font loading API for Safari.

@yurydelendik
Copy link
Contributor

yurydelendik commented Aug 31, 2017

And that's exactly what the Box fix seems to consist of as well: disabling the font loading API for Safari.

We can introduce a workaround to disable font loading for some browsers. Using disableFontFace looks too extentsive. I would just suggest to check if it's a Safari at https://github.com/mozilla/pdf.js/blob/master/src/display/font_loader.js#L301 to just disable native font loading or add a flag similar to disableFontFace or disableRange (we blacklist Safari there as well, see https://github.com/mozilla/pdf.js/blob/master/src/shared/compatibility.js#L626).

P.S. we can see https://bugs.webkit.org/show_bug.cgi?id=172210 also present in Chrome

@rijk
Copy link

rijk commented Aug 31, 2017

Actually, PDFJS.disableFontFace = true seems to work! I don't know what the downsides are (performance I guess?), but at least this gives us a short-term workaround.

@yurydelendik
Copy link
Contributor

yurydelendik commented Aug 31, 2017

Actually, PDFJS.disableFontFace = true seems to work! I don't know what the downsides are (performance I guess?), but at least we have a short-term workaround with this.

PDFJS.disableFontFace = true enables low quality internal font rendering engine -- no subpixel-AA and increased memory-CPU usage. It's a last resort option when font-faces are disabled by browser. We have intermediate fallback to load font without Font Loading API for legacy browser -- it shall be used here.

@rijk
Copy link

rijk commented Sep 21, 2017

Note that this issue appears to be resolved in Safari 11.

@timvandermeij
Copy link
Contributor

Closing since this issue is fixed upstream.

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

No branches or pull requests