There is some garbage on EMF while converting PDF to EMF

RESOLVED FIXED in Firefox 56

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: fatseng, Assigned: fatseng)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

No description provided.
Assignee

Updated

2 years ago
Blocks: pdf-printing
While printing the form 1040(https://www.irs.gov/pub/irs-pdf/f1040.pdf), PDFium generates some garbage in upper left corner. I marked them by red rectangles. Please see attachment.
When I launch Chrome by the no-sandbox flag, it also outputs the same result.
The garbage on upper left corner. I marked them by red rectangles.
Assignee

Updated

2 years ago
Assignee: nobody → fatseng
Assignee

Updated

2 years ago
Status: NEW → ASSIGNED
I found that Chromium doesn't use GDIPlusExt in PDFium while sandbox is enabled.
Summary: [Mortar] [pdfium] There is some garbage on EMF while converting PDF to EMF → There is some garbage on EMF while converting PDF to EMF
Comment on attachment 8886529 [details]
Bug 1367948 - Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering.

Create a patch file to avoid using GDIPlusExt while rendering EMF.
Add the patch to update.py to modify the pdfium source automatically.
Attachment #8886529 - Flags: feedback?(brsun)
Comment on attachment 8886530 [details]
Bug 1367948 - Part2. Prevent PDFium from using GDIPlusExt to avoid garbage rendering.

We force to |return false| here to avoid using GDIPlusExt.
Attachment #8886530 - Flags: feedback?(brsun)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8886529 [details]
Bug 1367948 - Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering.

https://reviewboard.mozilla.org/r/157340/#review162494

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:24
(Diff revision 1)
> +   return false;
> + }
> + 
> + bool IsGDIEnabled() {
> ++  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> ++  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt

GDIPlusExt is not a common term. Suggest to rephrase this term with your intention, depending on whether you want to refer to the CGdiplusExt class within PDfium, or you want to refer to some specific GDI+ and GDI32 functions of Windows.

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:25
(Diff revision 1)
> + }
> + 
> + bool IsGDIEnabled() {
> ++  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> ++  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt
> ++  // in PDFium and make sure that we go the same way as Chromium.

Please also help to add comments about what would go wrong if we don't do this tricky stuff.

Comment 9

2 years ago
mozreview-review
Comment on attachment 8886530 [details]
Bug 1367948 - Part2. Prevent PDFium from using GDIPlusExt to avoid garbage rendering.

https://reviewboard.mozilla.org/r/157342/#review162496

::: modules/pdfium/pdfium/core/fxge/win32/fx_win32_device.cpp:84
(Diff revision 1)
>    return false;
>  }
>  
>  bool IsGDIEnabled() {
> +  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> +  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt

ditto

::: modules/pdfium/pdfium/core/fxge/win32/fx_win32_device.cpp:85
(Diff revision 1)
>  }
>  
>  bool IsGDIEnabled() {
> +  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> +  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt
> +  // in PDFium and make sure that we go the same way as Chromium.

ditto
Comment on attachment 8886529 [details]
Bug 1367948 - Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering.

f+ if comments would be addressed.
Attachment #8886529 - Flags: feedback?(brsun) → feedback+
Comment on attachment 8886530 [details]
Bug 1367948 - Part2. Prevent PDFium from using GDIPlusExt to avoid garbage rendering.

f+ if comments would be addressed.
Attachment #8886530 - Flags: feedback?(brsun) → feedback+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8872858 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Priority: -- → P3
Comment on attachment 8886529 [details]
Bug 1367948 - Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering.

Using GDIPlusExt class in PDFium would render some garbage on EMF(attachment 8871552 [details]). Since Chromium runs in sandboxed process, it can't get screen DC and always returns |false| from IsGDIEnabled(). I tried to disable sandbox from Chromium. It also printed out some garbage.
I force to return false in IsGDIEnabled() to avoid using GDIPlusExt class within PDFium and make sure that we go the same way as Chromium.
Attachment #8886529 - Flags: review?(jwatt)
Assignee

Updated

2 years ago
Attachment #8886530 - Flags: review?(jwatt)

Comment 15

2 years ago
mozreview-review
Comment on attachment 8886529 [details]
Bug 1367948 - Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering.

https://reviewboard.mozilla.org/r/157340/#review165668

::: commit-message-201cb:1
(Diff revision 2)
> +Bug 1367948 - Part1. Add a patch file for PDFium

Make this "Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering."

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:7
(Diff revision 2)
> +# User Farmer Tseng <fatseng@mozilla.com>
> +# Date 1500268123 -28800
> +#      Mon Jul 17 13:08:43 2017 +0800
> +# Node ID 9f50f06ed643bcffff57dbfd9c176685f2554ccc
> +# Parent  4110bc73506d3e8f5d21479fe79ed91781f0300d
> +Bug 1367948 - Part2. Avoid using GDIPlusExt in PDFium

Make this comment "Prevent PDFium from using GDIPlusExt to avoid garbage rendering."

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:24
(Diff revision 2)
> +   return false;
> + }
> + 
> + bool IsGDIEnabled() {
> ++  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> ++  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt

Remove the words "force to", and add the word "the" before the word "GDIPlusExt".

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:25
(Diff revision 2)
> + }
> + 
> + bool IsGDIEnabled() {
> ++  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> ++  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt
> ++  // class within PDFium and make sure that we go the same way as Chromium.

Change "and make sure that we go the same way as Chromium" to "in order to take the same code paths as Chromium".

::: modules/pdfium/patches/bug1367948_avoid_using_GDIPlusExt.patch:26
(Diff revision 2)
> + 
> + bool IsGDIEnabled() {
> ++  // In Chromium, PDFium runs in sandboxed process which can't get the screen
> ++  // DC. Therefore, we force to |return false| here to avoid using GDIPlusExt
> ++  // class within PDFium and make sure that we go the same way as Chromium.
> ++  // If | return true |, PDFium would render some garbage on EMF, see Bug1367948

Replace this line with "This avoids an issue where PDFium can render some garbage to EMF (see bug 1367948)."
Attachment #8886529 - Flags: review?(jwatt) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8886530 [details]
Bug 1367948 - Part2. Prevent PDFium from using GDIPlusExt to avoid garbage rendering.

https://reviewboard.mozilla.org/r/157342/#review165670
Attachment #8886530 - Flags: review?(jwatt) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 22

2 years ago
Pushed by fatseng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ac08e5a6a6cf
Part1. Add a patch file to patch PDFium to prevent it from using GDIPlusExt to avoid garbage rendering. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/6abd15012dcd
Part2. Prevent PDFium from using GDIPlusExt to avoid garbage rendering. r=jwatt

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ac08e5a6a6cf
https://hg.mozilla.org/mozilla-central/rev/6abd15012dcd
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.