Bug 278675 – REGRESSION(275829@main): [GTK][WPE] Images broken in various PDFs

• Component: PDF
• Status: RESOLVED FIXED

[Michael Catanzaro]

This is the same as bug #271726, but for GTK/WPE. Unfortunately still we need the JPEG 2000 support for images to work properly in many PDFs. There is a sample PDF attached to bug #271726.

[Michael Catanzaro]

Plan:

 * Revert 273978@main, bring back OpenJPEG for now
 * Future improvement: ideally try to limit JPEG 2000 to PDF documents only rather than allowing it on the entire web
 * Future improvement: try building OpenJPEG for wasm as part of the PDF.js build process, bug #273435, so we can remove system OpenJPEG dependency again

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
>  * Revert 273978@main, bring back OpenJPEG for now

Bad plan. After attempting this, I spent a while trying to figure out why images in PDF documents were still broken. Turns out, this commit actually didn't break anything.

The only other suspicious commit is 277629@main, but that also didn't break things.

I will need to bisect this.

[Michael Catanzaro]

Wow, we actually have multiple bugs.

Test document one is https://bug-271726-attachments.webkit.org/attachment.cgi?id=470613

Test document two is https://insurance.mo.gov/earthquake/documents/EarthquakeInsuranceMarketsInMissouriReport20197-8-2019_000.pdf

They did not break at the same time. At the first step of my bisect, I can display test document one, but not test document two. :S

So now I'll bisect twice.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> Test document two is
> https://insurance.mo.gov/earthquake/documents/
> EarthquakeInsuranceMarketsInMissouriReport20197-8-2019_000.pdf

Unfortunately this broke in 275829@main "[GTK] Make it possible to build with Skia instead of Cairo".

Let's focus this bug report on this issue. I will create a separate bug report for the problem with test document one.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #4) 
> Unfortunately this broke in 275829@main "[GTK] Make it possible to build
> with Skia instead of Cairo".

Building with -DUSE_CAIRO=ON does *not* solve the problem, which I suppose is unsurprising, since that commit does not yet actually switch the default from USE_CAIRO to USE_SKIA. I have no guesses as to what aspect of that change breaks PDF images.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #4) 
> Let's focus this bug report on this issue. I will create a separate bug
> report for the problem with test document one.

Test document one displays fine in my development build. It is only broken in Ephy Tech Preview. So I cannot bisect it.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #5)
> Building with -DUSE_CAIRO=ON does *not* solve the problem, which I suppose
> is unsurprising, since that commit does not yet actually switch the default
> from USE_CAIRO to USE_SKIA. I have no guesses as to what aspect of that
> change breaks PDF images.

Well today it most certainly does. I made a stupid mistake: the default value of CMake variables doesn't change unless you delete your build directory, and I didn't do that when bisecting.

So that's good news actually. To avoid the regression, we can simply switch back to USE_CAIRO=ON and USE_SKIA=OFF. That would be a quick/easy solution for 2.46.

[Carlos Garcia Campos]

Even better news is that https://github.com/WebKit/WebKit/pull/33122 fixes the issue

[Michael Catanzaro]

Nice, thanks for investigating!

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> Wow, we actually have multiple bugs.
> 
> Test document one is
> https://bug-271726-attachments.webkit.org/attachment.cgi?id=470613
> 
> Test document two is
> https://insurance.mo.gov/earthquake/documents/
> EarthquakeInsuranceMarketsInMissouriReport20197-8-2019_000.pdf

Both are fixed by 283563@main.