810815 - Integrate pdf.js to Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

It would be very nice to be able read pdf files inline.

Comment 1

[Jonathan Protzenko [:protz]]

I spent one our two nights trying to make that work out. I somehow managed to make double-clicking on an attachment open up a pdf.js content tab, but then pdf.js was unable to download the attachment because it's an imap:// URL and there's some hackery involved. Conversations bundle a pdf.js-based viewer, which does have the hackery, but conversely, does not have the full-fledged PDF.js UI, unfortunately.

All in all, I don't think it's that much work required, but it probably requires some pdf.js knowledge which I don't have. And also, it was quite some time ago :)

Comment 2

[:aceman]

Is pdf.js even ready? It seems it was postponed from some FF versions. But I see it is enabled now in Aurora 18.

Comment 3

[Joshua Cranmer [:jcranmer]]

(In reply to :aceman from comment #2)
> Is pdf.js even ready? It seems it was postponed from some FF versions. But I
> see it is enabled now in Aurora 18.

It's stable enough to handle most of the PDFs I have to read (admittedly, mostly just academic papers, so not a strong indicator of general reliability).

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

IIRC pdf.js wasn't enabled in < FF18 because Gecko lacked some printing related features which
pdf.js needs.

Comment 5

[Jonathan Protzenko [:protz]]

From what I recall, pdf.js runs as a content script, but in the context of Thunderbird, you need to run with chrome privileges to download the contents of an attachment (which does indeed make a lot of sense!). So what is needed is to add an extra wrapper that fetches the contents of the attachment, and then passes it to pdf.js, for instance through a string in the URL, bypassing the default method for downloading a PDF (which is based on an XMLHttpRequest I think).

Comment 6

[Magnus Melin [:mkmelin] (away until July 8)]

Do you have a patch for that WIP experiment?

Comment 7

[Philip Chee]

CC some pdf.js developers

(In reply to Magnus Melin from comment #6)
> Do you have a patch for that WIP experiment?
Ping Protz

Comment 8

[Jonathan Protzenko [:protz]]

The code is in Conversations: https://github.com/protz/GMail-Conversation-View/tree/master/content/pdfviewer

There's an outer, chrome XUL window here: https://github.com/protz/GMail-Conversation-View/blob/master/content/pdfviewer/wrapper.js

It wraps the PDF contents as strings, which it then passes into the inner, HTML content window by calling its init function: https://github.com/protz/GMail-Conversation-View/blob/master/content/pdfviewer/viewer.js#L131

The inner HTML window then does the pdf.js incantations that allow the PDF to be rendered.

I completely gave up on adapting the addon "as-is"; I don't think I got very far anyway, and the code changed so much back then that it was pretty difficult to keep up with it. What I have now is a suboptimal viewer bundled with Conversations, but well, that does the job for now!

Comment 9

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Add pdfjs reader to Thunderbird.

This adds a copy of pdfjs into mail/extensions. The copy is taken from the existing version in m-c's browser/extensions.

It also adds the two pdf files.

Comment 10

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Hook up pdfjs files so that web tabs can be opened with pdfs.

This patch is enough that loading pdf files in web / special tabs works correctly. For example, from a WebExtension I can do browser.tabs.create({ "url": "http://.../mypdf.pdf" }); and it loads correctly.

I think preferences may be slightly broken here. Need to figure out how to add the "Preview in Thunderbird" option that's the same as the Firefox one (I'm currently looking at that), and at how the defaults are set for that option.

Comment 11

[Mark Banner (:standard8)]

For opening a pdf from a message, I have briefly tried it and hit what may have been the same issue as Jonathan had previously. Not quite sure as I don't have the full details.

We could try doing that part here, or in bug 766901 - depends if it is worth trying to land the web page parts of the functionality first (e.g. maybe for WebExtensions to start using?).

Comment 12

[Richard Marti (:Paenglab)]

Instead of copying the files couldn't we directly reference them from browser in the jar.mn? Then we don't need to copy them every time they are updated in browser?

Comment 13

[Magnus Melin [:mkmelin] (away until July 8)]

That's not how we do it, we only include directly if in toolkit. Maybe pdfjs should be in toolkit?

Comment 14

[Richard Marti (:Paenglab)]

But it wouldn't be the first: https://searchfox.org/comm-central/search?q=..%2F..%2F..%2Fbrowser&case=false&regexp=false&path=mail and we've got automatically the newest version.

Comment 15

[Mark Banner (:standard8)]

It seems that either will probably work. I'm not sure if m-c would move pdfjs into toolkit as I'm guessing it isn't really relevant to mobile.

Comment 16

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Add pdfjs reader locale files to Thunderbird.

I've changed the main patch to reference browser/ directly. I have no idea if it is possible with locale files, so I've just copied those across. If someone wants to play around with that, feel free to, but I'm not planning on it.

Comment 17

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Hook up pdfjs files so that web tabs can be opened with pdfs (disabled by default).

This hooks up pdfjs and gets the menu option working in preferences, but disables it by default. If you try to enable it and open a pdf from an email, then you get an error (as described previously).

In a WebExtension, I've been doing this to test it:

browser.tabs.create({
  url: "http://www.orimi.com/pdf-test.pdf",
});

(the url is just a random test pdf I found).

The code for the preferences is largely copied from the browser, adapted to non-fluent. I did consider porting some of the fluent work, but that's too much for what I want to do.

I also looked at porting browser/components/preferences/in-content/tests/browser_applications_selection.js, however again the lack of fluent made this difficult.

I'm planning to look into the error you get when opening a pdf from an email, but thought it is worth starting the review process, and possibly landing these so that extensions can start to at least use parts of it.

Comment 18

[Magnus Melin [:mkmelin] (away until July 8)]

I opened bug 1618465 about moving it to toolkit. I don't think we should reference browser/ directly, though we might need to reconsider in the future.

Comment 19

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9121415 [details] [diff] [review]
Bug 810815 - Hook up pdfjs files so that web tabs can be opened with pdfs (disabled by default).

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

Sorry for the delay. This looks alright except for how it's referencing browser

::: mail/components/MailGlue.jsm
@@ +19,5 @@
> +  MailMigrator: "resource:///modules/MailMigrator.jsm",
> +  PdfJs: "resource://pdf.js/PdfJs.jsm",
> +  RemoteSecuritySettings:
> +    "resource://gre/modules/psm/RemoteSecuritySettings.jsm",
> +  Services: "resource://gre/modules/Services.jsm",

I think we want to keep services imported normally, and looks like most of the others too will basically always be needed.

Comment 20

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9121413 [details] [diff] [review]
Bug 810815 - Add pdfjs reader locale files to Thunderbird.

Shouldn't be needed if it's in toolkit.

Comment 21

[Mark Banner (:standard8)]

I eventually worked out today why this wasn't fully working. The pdfjs loads itself, but pretends it is actually loading the url of the document. Since the document is based on the url of the message, we disable javascript on the docshell.

Ideally we shouldn't be doing that for this case, though I haven't yet found a way around it. Firefox has this issue when javascript is disabled globally as well (bug 866634), so it could be the fix needs to be in core somewhere.

I've been asking around in #dom on Matrix, hopefully might be able to get some help in getting that bit to work.

Comment 22

[Emilio Cobos Álvarez [:emilio]]

This should work with the two patches in bug 866634 and bug 1631305.

Comment 23

[Mark Banner (:standard8)]

Attachment: 810815.txt

This is my current WIP. Still needs some work and clean up.

Comment 24

[Mark Banner (:standard8)]

Thank you Emilio, it is working with those two patches applied :-)

I'll see if I can sort out the move to toolkit and get the patch updated over the next few days.

Comment 25

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Enable opening pdf attachments within Thunderbird.

With the patches in core that have landed, and the additional patch on bug 1618465, this now gets everything working for opening pdf attachments within Thunderbird using pdfjs.

Comment 26

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9143404 [details] [diff] [review]
Bug 810815 - Enable opening pdf attachments within Thunderbird.

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

Looks reasonable, but something's broken so I can't test the actual feature, see below.

::: mail/app/profile/all-thunderbird.js
@@ +1247,5 @@
>  #endif
>  
>  pref("prompts.defaultModalType", 3);
> +
> +// Completely disable pdf.js as an option to preview pdfs within firefox.

Thunderbird

@@ +1251,5 @@
> +// Completely disable pdf.js as an option to preview pdfs within firefox.
> +// Note: if this is not disabled it does not necessarily mean pdf.js is the pdf
> +// handler just that it is an option.
> +pref("pdfjs.disabled", false);
> +// Used by pdf.js to know the first time firefox is run with it installed so it

Thunderbird

::: mail/components/MailGlue.jsm
@@ +31,5 @@
>  );
>  var { RemoteSecuritySettings } = ChromeUtils.import(
>    "resource://gre/modules/psm/RemoteSecuritySettings.jsm"
>  );
> +var { PdfJs } = ChromeUtils.import("resource://pdf.js/PdfJs.jsm");

this throws an NS_ERROR_NOT_AVAILABLE

Which makes lots of other things not work. I get a tb with missing Get messages button and opening the compose window doesn't work.

::: mailnews/base/util/nsMsgProtocol.cpp
@@ +51,1 @@
>                    nsIRequestObserver, nsIRequest, nsITransportEventSink)

Looks like this needs a clang-format adjustment

Comment 27

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9143404 [details] [diff] [review]
Bug 810815 - Enable opening pdf attachments within Thunderbird.

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

Oh I see, bug 1618465 hasn't landed yet

Comment 28

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9143404 [details] [diff] [review]
Bug 810815 - Enable opening pdf attachments within Thunderbird.

Clearing request until bug 1618465 lands

Comment 29

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Enable opening pdf attachments within Thunderbird.

Updated patch now that the move has landed. I've not run this through try server, but it seems to work fine.

Comment 30

[Magnus Melin [:mkmelin] (away until July 8)]

Comment on attachment 9161019 [details] [diff] [review]
Bug 810815 - Enable opening pdf attachments within Thunderbird.

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

Looks good, thanks for pulling it through! r=mkmelin
I've fixed the below nits and sent it off to try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8db22b78a08d76fce855f3a49feb9a17d230eee7

::: mail/app/profile/all-thunderbird.js
@@ +1258,5 @@
>  
>  // The URL for the privacy policy related to recommended extensions.
>  pref("extensions.recommendations.privacyPolicyUrl", "https://www.mozilla.org/en-US/privacy/thunderbird/#addons");
> +
> +// Completely disable pdf.js as an option to preview pdfs within firefox.

Thunderbird.

@@ +1262,5 @@
> +// Completely disable pdf.js as an option to preview pdfs within firefox.
> +// Note: if this is not disabled it does not necessarily mean pdf.js is the pdf
> +// handler just that it is an option.
> +pref("pdfjs.disabled", false);
> +// Used by pdf.js to know the first time firefox is run with it installed so it

Thunderbird.

Comment 31

[Magnus Melin [:mkmelin] (away until July 8)]

The try shows bustage detected by static analysis.

Adding this should show it locally too:
ac_add_options --enable-clang-plugin

Comment 32

[Mark Banner (:standard8)]

Attachment: Enable opening pdf attachments within Thunderbird v2

Updated patch, fixed the refcnt issues (the NS_DECL_* macros needed adjusting).

Comment 33

[Mark Banner (:standard8)]

I can land this, though tree is closed at the moment, so flagging for landing when it re-opens (unless I get there first!).

Comment 34

[Mark Banner (:standard8)]

Attachment: Bug 810815 - Enable opening pdf attachments within Thunderbird. r=mkmelin!

Depends on D86476

Comment 35

[Phabricator Automation]

Comment on attachment 9169141 [details]
Bug 810815 - Enable opening pdf attachments within Thunderbird. r=mkmelin!

Revision D86538 was moved to bug 810814. Setting attachment 9169141 [details] to obsolete.

Comment 36

[Richard Marti (:Paenglab)]

patch had the wrong bug number. Patch is in bug 810814.

Comment 37

[Mark Banner (:standard8)]

Sorry about that, commit is: https://hg.mozilla.org/comm-central/rev/08e8b872695145ef325df8fee38139517d3f1be2

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f5690ed26dbe
Add a dummy gBrowser._insertBrowser test function to let mochitests pass. r=bustage-fix

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f8922777d979
Test fix for browser_policy_passwordmanager.js and for debug tests. r=bustage-fix