1028735 - PDF: Missing characters in formula

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P2)

Description

[hjp]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140616143923

Steps to reproduce:

I opened the PDF at http://www.wifo.ac.at/jart/prj3/wifo/resources/person_dokument/person_dokument.jart?publikationsid=47258&mime_type=application/pdf with the builtin PDF viewer.


Actual results:

On page 15, there are 3 equations. In all 3 equations, the "=" is missing (see screenshot)


Expected results:

The = signs should be displayed (as in the second screenshot, which shows the same part of the document displayed with okular).

Comment 1

[hjp]

Attachment: screenshot: FF builtin PDF viewer shows equations without "=" signs

Comment 2

[hjp]

Attachment: screenshot: okular shows equations with "=" signs

Comment 3

[Yury Delendik (:yury)]

Looks like it works in FF30

Comment 4

[Jonas Jenwald [:Snuffleupagus]]

This is a regression from https://github.com/mozilla/pdf.js/commit/b5b94a4af389ef387570cf22662d9fc6bd41417a (i.e. PR https://github.com/mozilla/pdf.js/pull/4259).

Comment 5

[Jonas Jenwald [:Snuffleupagus]]

Attachment: Reduced testcase

Comment 6

[Alice0775 White]

Regression window(m-c)
Good:
https://hg.mozilla.org/mozilla-central/rev/a4ac15d6912d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140404113226
Bad:
https://hg.mozilla.org/mozilla-central/rev/3831f8e22e30
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140404113925
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4ac15d6912d&tochange=3831f8e22e30

Regressed by:
	e1d196155712	Ryan VanderMeulen — Bug 990852 - Update pdf.js to version 0.8.1334. r=yury, r=Mossop

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Updating the track flags accordingly.
Tracking. "=" is sometimes used in PDF files.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Yury, any chance to see that bug fixed for 31? Beta 6 is going to be released today. We can accept a fix in beta 8 max for this (next monday). Thanks

Comment 9

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Yury, any chance to see that bug fixed for 31? Beta 6 is going to be
> released today. We can accept a fix in beta 8 max for this (next monday).
> Thanks

Hi Sylvestre, we're a little short-handed at the moment, Yury is on PTO for another week, I think.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Summary on this bug:
* Yury and Brendan are in PTO
* Yury comes back July 18th and we don't have anyone else until then
* We will release the 22nd

I don't see any other solutions than coming back to the state of 30. 
However, we updated pdf.js a few times: bug 990852,  bug 999616, bug 1026488...
Not sure what to do here...

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Unfortunately, this still reproduces with the browser add-on based off upstream pdf.js git tip (1.0.441).

So I think our best bet is seeing if we can find a pdf.js community member with the ability to investigate. A revert to the Fx30 version of pdf.js is also a possibility, but not entirely straightforward. If we have some time to look for a proper fix still, I think that's still preferable and less-messy.

Comment 12

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11)
> Unfortunately, this still reproduces with the browser add-on based off
> upstream pdf.js git tip (1.0.441).
> 
> So I think our best bet is seeing if we can find a pdf.js community member
> with the ability to investigate. A revert to the Fx30 version of pdf.js is
> also a possibility, but not entirely straightforward. If we have some time
> to look for a proper fix still, I think that's still preferable and
> less-messy.

Hi Ryan,

I agree completely, I'm over on #pdfjs on IRC recruiting people now, stand by for updates...

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Jonas was kind enough to send me a roll-up patch of the upstream pull request and I've submitted it to Try on top of current beta.
https://tbpl.mozilla.org/?tree=Try&rev=95b551769b3b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Attachment: cherry-picked patch

I can confirm that this patch from the Try push fixes the reduced testcase attached to this bug. No idea who should review it in Yury/Brandon's absences.

Dave, can you?

Comment 15

[Dave Townsend [:mossop]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #14)
> Created attachment 8452628 [details] [diff] [review]
> cherry-picked patch
> 
> I can confirm that this patch from the Try push fixes the reduced testcase
> attached to this bug. No idea who should review it in Yury/Brandon's
> absences.
> 
> Dave, can you?

Sorry, I really don't know this part of the code so I can't say anything about it

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Bill, who can look at this if Yury's on PTO until 7/20 and this realistically needs to land by the end of the week to make the Fx31 RC build?

Comment 17

[u279076]

As per Release Coordination meeting, I'm going to take a look at this to see how many PDFs I can trigger this with.

Comment 18

[Bill Walker [:bwalker] [@wfwalker]]

Yury says he may be online today or tomorrow, so I'll put the needinfo back to him...

Comment 19

[u279076]

I've been scouring the internet to try various PDFs (I think I've tried about 30 different PDFs so far) and have yet to encounter one that reproduces this bug. That said, I cannot seem to reproduce this with the attached URL in comment 0. I can only seem to reproduce this with the minimized testcase.

In addition, I asked Tyler to see if users are talking about this on our Beta feedback channels and he said he could find any mention of it. His team will be launching a snippet survey shortly so that might tell us more.

Based on this information, I do not think we should hold up release, nor should we back out this version of PDF.js. We should obviously take a fix if we have one, but let's not hold up release for it. 

In the meantime I'll keep looking for a PDF that reproduces this but my confidence isn't very high that I'll find one. If I don't report back that I've found one in the next hour assume that I haven't found one. Unfortunately I will need to move on to other, more critical issues at that point.

Comment 20

[Yury Delendik (:yury)]

Comment on attachment 8452628 [details] [diff] [review]
cherry-picked patch

Review of attachment 8452628 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16)
> Bill, who can look at this if Yury's on PTO until 7/20 and this
> realistically needs to land by the end of the week to make the Fx31 RC build?

The cherry-pick patch is fine (r+), but personally I would not risk uplift to the Fx31. I agree with assumption at https://github.com/mozilla/pdf.js/pull/5017 "... even though this PR does fix two regressions, there is obviously a risk that it can regress other PDF files...". Therefore I'm not seeking for uplift approval.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

OK. Thanks for the feedback during your PTO. It is appreciated!

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Yury, once you are back from PTO, could you request an uplift to aurora? Thanks

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Standard practice is to fix on trunk via upstream code imports. Given Yury's PTO situation, it's likely that it won't happen until after Monday's merge. I'm also not entirely sure if the attached patch is ultimately what we're going to want on the release branches given comment 20.

Comment 24

[[:Aleksej]]

attachment 8444849 [details]:
blank: 2014-06-16-03-02-03-mozilla-central-firefox-33.0a1.en-US.linux-x86_64
=:     2014-08-06-03-02-01-mozilla-central-firefox-34.0a1.en-US.linux-x86_64

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Yury, are you planning to ask for an uplift in 32 or 33? Or do you think it should just ride the train? thanks

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Bug 1042307 was uplifted to Aurora, so 33 is already fixed.

Comment 27

[Yury Delendik (:yury)]

Comment on attachment 8452628 [details] [diff] [review]
cherry-picked patch

Approval Request Comment
[Feature/regressing bug #]: bug 990852
[User impact if declined]: some pdf will not be displayed right
[Describe test coverage new/current, TBPL]: fix present in FF33 and 34
[Risks and why]: low, though might affect some other PDFs
[String/UUID change made/needed]: none

Comment 28

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8452628 [details] [diff] [review]
cherry-picked patch

We didn't like shipping this in 31. Good to have a fix for 32. Beta+

Comment 29

[Yury Delendik (:yury)]

There is some conflicts for the patch and mozilla-beta

Comment 30

[Yury Delendik (:yury)]

Attachment: cherry-picked patch for FF32

The same as attachment 8452628 [details] [diff] [review] but with resolved conflicts.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/599c7756380c

Comment 32

[[:Aleksej]]

VERIFIED FIXED with Aurora and Beta.

attachment 8444849 [details]:

Aurora:
blank: 2014-07-16-00-40-01-mozilla-aurora-firefox-32.0a2.en-US.linux-x86_64
=    : 2014-08-08-00-40-01-mozilla-aurora-firefox-33.0a2.es-ES.linux-x86_64
=    : 2014-08-09-00-40-01-mozilla-aurora-firefox-33.0a2.ru.linux-x86_64

Beta:
blank: firefox-32.0b4.en-US.linux64
blank: firefox-32.0b4.ru.linux64
=    : firefox-32.0b5.ru.linux64

Comment 33

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Verified as fixed also on Windows 7 x64, Windows 8 x64 and Mac OS X 10.7.5 with:
- Firefox 32 Beta 5 - BuildID: 20140807212602
- Latest Firefox 33 Aurora - BuildID: 20140811004001
- Latest Firefox 34 Nightly - BuildID: 20140811030203