837461 - pdf.js Black squares appear in some PDF documents even though the original PDFs does not have them.

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[rshimazu]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130130080006

Steps to reproduce:

I access to the following urls:

http://www.bowers-wilkins.com/Downloads/Product/Manual/Zeppelin_Air_Manual.pdf#page=128
http://www.namura.cc/air/AIR_manual.pdf
http://aitech.ac.jp/lib/kiyou/40B/B17.pdf
http://www.alpha-web.jp/service/internet/roaming/pdf/wlan-ap04.pdf


Actual results:

Black squares(boxes), which are not included in the original document, and are appearently strange, appear in the document. 


Expected results:

Squares should not appear.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

WFM using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 ID:20130130080006 and

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130202 Firefox/21.0 ID:20130202030955 + PDF.js 0.7.142

Comment 2

[Loic]

Could you take a screenshot of these black boxes, please.

In addition, try with hardware acceleration disabled: http://support.mozilla.org/en-US/kb/forum-response-disable-hardware-acceleration

Comment 3

[rshimazu]

Attachment: Please see the red circle.

Comment 4

[rshimazu]

Attachment: Please see the red circle(2).

Comment 5

[rshimazu]

I disabled hardware acceleration, but no change. Still I see obvious strangeness.

Comment 6

[Loic]

Could you try with a fresh profile, please: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

Comment 7

[rshimazu]

I created a new profile and tested. But no change and the same result.

Comment 8

[Masatoshi Kimura [:emk]]

I saw this on the latest Nightly, but not on Firefox 18.

Comment 9

[Loic]

Confirmed too, but only with HWA disabled (on Win 7).
Could you confirm that HWA is disabled when you're seeing the black/gray rectangles? (type about:support > graphics)

Regression range:
m-c
good=2012-11-04
bad=2012-11-05
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c6b71348e20&tochange=0947e291578a

Comment 10

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/ab099c9e1a09
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121104012747
Bad:
http://hg.mozilla.org/mozilla-central/rev/2937fd8e35a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121104152246
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ab099c9e1a09&tochange=2937fd8e35a1

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/38b97d2a1bc5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121103200445
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/58b4bd7b5065
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121104025144
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=38b97d2a1bc5&tochange=58b4bd7b5065

Suspected:
	58b4bd7b5065	Jonathan Kew — bug 808288 - render missing glyphs as hexboxes in <canvas> text. r=bas

Comment 11

[Jonathan Kew [:jfkthame]]

I'm not able to reproduce this on Win7 or OS X; will try WinXP shortly.

However, I do notice a number of font-related errors in the Web Console when loading these PDFs, including some fonts that are rejected; this implies that the rendering is probably falling back to a locally-installed font instead of using the embedded font from the PDF document.

This suggests that the root of the problem here may well be pdf.js font handling, and the change in bug 808288 just caused the "junk" that it is drawing to become visible instead of invisible. But it shouldn't be drawing text to the canvas without the proper font (or is it an encoding problem?) in the first place.

Comment 12

[Alice0775 White]

(In reply to Jonathan Kew (:jfkthame) from comment #11)
> I'm not able to reproduce this on Win7 or OS X; will try WinXP shortly.
> 
See comment #9, It only happens when HWA disabled.

Comment 13

[Jonathan Kew [:jfkthame]]

Ah, right - sorry, I overlooked that. Also reproduced on my WinXP VM, which makes sense, as we use the GDI font backend there.

I notice that with Firefox 18 on XP, I see missing-glyph "boxes" where there are spaces in the English fragments of text. With Nightly, I see those ugly gray bars instead, but those spaces are the trigger for it. I'm not sure yet why they appear so long, but I think there's some kind of font-handling or encoding problem in pdf.js that is causing it to try to draw an undefined character for those spaces.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Jonathan, can you tell us whether a backout of bug 808288 is feasible here and what state that would leave us in when we ship 19?  The black boxes are very undesireable, can we still ship pdf.js with the former missing glyph behaviour?

Comment 15

[Jonathan Kew [:jfkthame]]

Completely backing out 808288 isn't a very attractive option, as the old canvas code was broken to the extent that it rendered some arbitrary ("random") glyph in place of any missing character. Much of the time, that might produce nothing visible, but it also had the potential to draw a totally -wrong- glyph. (See the testcase and screenshot in bug 808288.)

But as it happens, bug 808288 was implemented in several logical steps, so one option would be to back out the second and third patches there, leaving only the first stage. That was a very small patch to make canvas text simply leave missing characters blank, not attempt to draw a glyph at all.

I think that would be a very safe option, and the result for pdf.js should be as good as it can be short of actually fixing pdf.js such that it renders the -correct- glyph from the correct font.

I'll push a tryserver job of mozilla-beta with such a backout (i.e. back out bug 808288 parts 2 and 3) to confirm that it works as expected.

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: backout parts 2 and 3 of bug 808288 because pdf.js may paint missing chars to canvas if it failed to load fonts, which looks really ugly; better to skip them for now.

This is the partial backout, as described above. Tryserver run in progress at https://tbpl.mozilla.org/?tree=Try&rev=78403bf0e966. I'm expecting it to fail a reftest on some OS X systems; if we land this, we'll need to readjust the manifest accordingly.

Comment 17

[Jonathan Kew [:jfkthame]]

The tryserver job above was pushed from a mozilla-beta tree, so some builds such as b2g may fail.

Also pushed a job with the same partial backout based on mozilla-central, to confirm desktop reftest results there:

https://tbpl.mozilla.org/?tree=Try&rev=82ef89255a0d

Comment 18

[Jonathan Kew [:jfkthame]]

From tracing the behavior when rendering the PDF documents from comment #0, it's clear that the underlying issue here is that pdf.js is failing to create a usable font resource from one or more of the embedded fonts in the document.

The font(s) are either rejected by the OTS sanitizer, or fail to activate on the platform; this shows up in the Web Console, as we log error messages there for font failures.

Then pdf.js proceeds to draw text using Private Use Area codepoints (U+Exxx, at least in the cases I traced), which were intended to map to specific glyphs in the embedded font but are not supported by whatever standard font we fall back to, due to the font-loading failure. And so we hit the missing-glyph code path, which on current trunk would usually be expected to draw hexboxes. I'm guessing that the ugly black bars we're seeing are a fragment clipped from those hexboxes, which are being drawn at a huge size due to some quirk of how pdf.js is dealing with font scaling, though I'm not 100% sure of this.

The suggested partial-backout here can't solve the problem of pdf.js failing to create a usable font resource, but by making missing glyphs entirely invisible in canvas, it will at least make the result less glaringly bad. (It's not really a good solution, though, as users might not even realize that some of the text in their document isn't showing up.)

Comment 19

[Jonathan Kew [:jfkthame]]

Tryserver results are looking ok... I assume the android test failures are because of pushing mozilla-beta (rather than trunk) to tryserver, and things have changed too much for this to work. The canvas/text-emoji.html reftest failure makes perfect sense, as most platforms don't have a font for the character used there and so this hits the canvas missing-character code, which is exactly what we're proposing to back out here. If we take this backout patch, I'll mark this in the reftest manifest.

Comment 20

[Jonathan Kew [:jfkthame]]

Pushed to m-c, including reftest manifest update to mark text-emoji.html as random on most platforms.
https://hg.mozilla.org/mozilla-central/rev/8fc06e61119a

Comment 21

[Jonathan Kew [:jfkthame]]

Comment on attachment 710914 [details] [diff] [review]
backout parts 2 and 3 of bug 808288 because pdf.js may paint missing chars to canvas if it failed to load fonts, which looks really ugly; better to skip them for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not strictly a regression, but bug 808288 results in visually worse results due to pdf.js font-loading issues

User impact if declined: potential for really ugly rendering of some PDFs on Windows GDI systems, when loading of document's embedded fonts fails

Testing completed (on m-c, etc.): verified locally that mozilla-beta tryserver build (comment 16) renders as expected - missing glyphs are simply absent, no black blobs; landed on mozilla-central

Risk to taking this patch (and alternatives if risky): minimal risk, this just removes the code that was added to draw missing-glyph hexboxes in canvas text

String or UUID changes made by this patch: none

Comment 22

[Alex Keybl [:akeybl]]

Comment on attachment 710914 [details] [diff] [review]
backout parts 2 and 3 of bug 808288 because pdf.js may paint missing chars to canvas if it failed to load fonts, which looks really ugly; better to skip them for now.

Low risk partial backout that will improve the rendering of non-English PDFs. Approving for Aurora/Beta.

Comment 23

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/08d9367224c5
https://hg.mozilla.org/releases/mozilla-beta/rev/0c688282d11f

Comment 24

[Mihai Morar, (:MihaiMorar)]

I confirm the fix is verified on Latest Aurora on Windows Vista x64:

Mozilla/5.0 (Windows NT 6.0; WOW64; rv:20.0) Gecko/20130210 Firefox/20.0(20130210042017)

I will make same investigations on FF 19b6 because on FF 19b5 it was not implemented yet.

Comment 25

[Mihai Morar, (:MihaiMorar)]

I confirm fix is verified on FF 19b6 and Latest Nightly too on Windows Vista x64

Mozilla/5.0 (Windows NT 6.0; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 (20130212082553)

Mozilla/5.0 (Windows NT 6.0; WOW64; rv:21.0) Gecko/20130212 Firefox/21.0(20130212031120)

Comment 26

[rshimazu]

I am the original reporter of this bug. I also confirmed the fix on Firefox 19b6 with pdf.js 0.7.234 addon. Thank you.

Comment 27

[Mihai Morar, (:MihaiMorar)]

(In reply to rshimazu from comment #26)
> I am the original reporter of this bug. I also confirmed the fix on Firefox
> 19b6 with pdf.js 0.7.234 addon. Thank you.

Thank you for confirming too.

Comment 28

[RonB]

Would anyone care to see if this bug is related to https://bugzilla.mozilla.org/show_bug.cgi?id=858128?

Comment 29

[Loic]

(In reply to RonB from comment #28)
> Would anyone care to see if this bug is related to
> https://bugzilla.mozilla.org/show_bug.cgi?id=858128?

No, bug 858128 is still present in FF23.