PDF: Missing characters in formula

VERIFIED FIXED in Firefox 32

Status

()

P2
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: hjp, Assigned: Snuffleupagus)

Tracking

({regression})

31 Branch
Firefox 34
x86
All
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox30 unaffected, firefox31+ wontfix, firefox32+ verified, firefox33+ verified, firefox34 verified, relnote-firefox 31+)

Details

(Whiteboard: [pdfjs-c-rendering][pdfjs-d-font-conversion][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/5017)

Attachments

(5 attachments)

(Reporter)

Description

5 years ago
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).
(Reporter)

Comment 2

5 years ago
Component: General → PDF Viewer
Product: Core → Firefox
Looks like it works in FF30
Keywords: regression, regressionwindow-wanted
Priority: -- → P2
Whiteboard: [pdfjs-c-rendering][pdfjs-d-font-conversion]
(Assignee)

Comment 5

5 years ago
Posted file Reduced testcase
tracking-firefox31: --- → ?
tracking-firefox32: --- → ?
tracking-firefox33: --- → ?

Comment 6

5 years ago
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
Blocks: 990852
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regressionwindow-wanted
OS: Linux → All
Updating the track flags accordingly.
Tracking. "=" is sometimes used in PDF files.
status-firefox30: --- → unaffected
status-firefox31: --- → affected
status-firefox32: --- → affected
status-firefox33: --- → affected
tracking-firefox31: ? → +
tracking-firefox32: ? → +
tracking-firefox33: ? → +
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
Flags: needinfo?(ydelendik)
(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.
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...
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.
(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...
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
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?
Flags: needinfo?(dtownsend+bugmail)
(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
Flags: needinfo?(dtownsend+bugmail)
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?
Flags: needinfo?(ydelendik) → needinfo?(bwalker)
As per Release Coordination meeting, I'm going to take a look at this to see how many PDFs I can trigger this with.
Keywords: qawanted
QA Contact: anthony.s.hughes
Yury says he may be online today or tomorrow, so I'll put the needinfo back to him...
Flags: needinfo?(bwalker) → needinfo?(ydelendik)
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.
Keywords: qawanted
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.
Attachment #8452628 - Flags: review+
OK. Thanks for the feedback during your PTO. It is appreciated!
status-firefox31: affected → wontfix
Yury, once you are back from PTO, could you request an uplift to aurora? Thanks
Keywords: checkin-needed
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.
Assignee: nobody → jonas.jenwald
Keywords: checkin-needed
Whiteboard: [pdfjs-c-rendering][pdfjs-d-font-conversion] → [pdfjs-c-rendering][pdfjs-d-font-conversion] https://github.com/mozilla/pdf.js/pull/5017
relnote-firefox: --- → 31+
Depends on: 1042307
Flags: needinfo?(ydelendik)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34

Comment 24

5 years ago
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
QA Whiteboard: [bugday-20140806]
Yury, are you planning to ask for an uplift in 32 or 33? Or do you think it should just ride the train? thanks
Flags: needinfo?(ydelendik)
status-firefox34: --- → fixed
Bug 1042307 was uplifted to Aurora, so 33 is already fixed.
status-firefox33: affected → fixed
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
Attachment #8452628 - Flags: approval-mozilla-beta?
Flags: needinfo?(ydelendik)
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+
Attachment #8452628 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: checkin-needed
Whiteboard: [pdfjs-c-rendering][pdfjs-d-font-conversion] https://github.com/mozilla/pdf.js/pull/5017 → [pdfjs-c-rendering][pdfjs-d-font-conversion][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/5017, checkin needed "cherry-picked" patch for FF32
Keywords: checkin-needed
There is some conflicts for the patch and mozilla-beta
Whiteboard: [pdfjs-c-rendering][pdfjs-d-font-conversion][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/5017, checkin needed "cherry-picked" patch for FF32 → [pdfjs-c-rendering][pdfjs-d-font-conversion][pdfjs-f-fixed-upstream] https://github.com/mozilla/pdf.js/pull/5017
The same as attachment 8452628 [details] [diff] [review] but with resolved conflicts.
Keywords: verifyme

Comment 32

5 years ago
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
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
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
status-firefox33: fixed → verified
status-firefox34: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.