1193155 - [non-e10s] Selecting applications->Portable document format (PDF) -> Use Adobe Acrobat (in Firefox) needs restart before becoming effective

Status: RESOLVED WORKSFORME

(Firefox :: File Handling, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]: Regression since 40.0candidate

+++ This bug was initially created as a clone of Bug #1193088 +++

This bug is reproduced with disabled e10s. (about in e10s mode, see Bug 1193088).

If I set the option (Tools -> Options) to open pdf documents in the Adobe Acrobat plugin preview (in firefox),
this options becomes only effective after restarting firefox. 

Selecting this option without restart results in a download of the pdf file (like selecting the option "safe file")


Example: http://website-archive.mozilla.org/www.mozilla.org/security/security/iSECPartners_Phishing.pdf


Steps to reproduce:
1. Start with disabled e10s
2. Open pdf
   --- open with pdf.js --- as default expected

3. set the option (Tools -> Options) to open pdf documents in the Adobe Acrobat plugin preview (in firefox) 
4. Open pdf
   --- Bug

5. Restart Browser
6. Open pdf
   --- open with Adobe Acrobat plugin preview in a tab  --- as default expected

Actual results:
Not open witn Adobe Acrobat plugin.
Something start download.

Expected results;
The pdf should shown in Adobe Acrobat plugin inside tab without restart.


Regression window
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=aaae8916f107&tochange=9a9a92d6d78b

Regressed by:
9a92d6d78b	Yury Delendik — Bug 1179262 - Remove PlayPreview usage from PDF viewer. r=bz

Comment 1

[Andreas Jung]

"needs reload" in the bug title is misleading and should be changed to "needs restart"

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked since this is a regression.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Michael, do you know who could help with this bug? Thanks
It is too late for 40 but we could take a fix for 41.

Comment 4

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

It is getting too late to fix this in FF41 given that we don't have a patch ready. This doesn't seem to be a release blocker for 41.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

I agree that the user experience could be better here, I don't think it is worth tracking it.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Boris, as you reviewed the patch causing this regression, maybe you have an idea on how to fix it. Thanks

Comment 7

[Boris Zbarsky [:bzbarsky]]

Presumably what needs to happen is that changing the pref needs to register/unregister the pdfjs stuff accordingly.  I do see preference observers and observer service observers in PdfJs.jsm and at first glance they're hooked up reasonably.  Someone who is familiar with that code should step through what happens when the steps for this bug are followed and see whether the right logic is running.  I can try to do that, but I have a ton of other things on my plate right now, so it might be better if someone who actually knows this stuff looks at it.  If that's not an option, I can look sometime next week.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Bz, as I don't think this is a real critical issue, you can take your time to have a look to this one.

I will mark 42 as wontfix.

Comment 9

[Yury Delendik (:yury)]

The nsComponentManagerImpl::UnregisterFactory does not remove a factory from contracts list which causes nsContentUtils::IsPDFJSEnabled return true. The UnregisterFactory behavior is not intuitive since nsComponentManagerImpl::RegisterFactory adds contractID-to-factory mapping. At the moment, I'm trying to understand how to properly remove the contractID mapping introduced by the RegisterFactory. There is also the "RegisterFactory then UnregisterFactory can leave an entry in mContractIDs pointing to an unusable nsFactoryEntry." comment, however it is not really helpful.

Comment 10

[Joachim Herb]

The fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1217047 also fixes this bug. 

So will that fix also be added to Firefox 43 beta or Firefox 44a2?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Nathan, see comment 10?

Comment 12

[Nathan Froyd [:froydnj]]

(In reply to Boris Zbarsky [:bz] from comment #11)
> Nathan, see comment 10?

I requested aurora and beta uplift in the other bug, since it actually has the patch.

Comment 13

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

The fix for bug 1217047 has now been uplifted to Aurora and Beta, and should now be in Firefox 43 Beta 4. Alice, do you want to verify it?

Comment 14

[Alice0775 White]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #13)
> The fix for bug 1217047 has now been uplifted to Aurora and Beta, and should
> now be in Firefox 43 Beta 4. Alice, do you want to verify it?

I can verify that beta43.0b4 fixes the problem.