Closed Bug 1193155 Opened 7 years ago Closed 6 years ago

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

Categories

(Firefox :: File Handling, defect)

40 Branch
All
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox39 --- unaffected
firefox40 + wontfix
firefox41 + wontfix
firefox42 - wontfix
firefox43 --- fixed
firefox44 --- fixed
firefox45 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected

People

(Reporter: alice0775, Unassigned)

References

Details

(Keywords: regression)

[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
Flags: needinfo?
Flags: needinfo? → needinfo?(ydelendik)
"needs reload" in the bug title is misleading and should be changed to "needs restart"
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.
Flags: needinfo?(mbebenita)
Summary: [non-e10s] Selecting applications->Portable document format (PDF) -> Use Adobe Acrobat (in Firefox) needs reload before becoming effective → [non-e10s] Selecting applications->Portable document format (PDF) -> Use Adobe Acrobat (in Firefox) needs restart before becoming effective
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.
I agree that the user experience could be better here, I don't think it is worth tracking it.
Boris, as you reviewed the patch causing this regression, maybe you have an idea on how to fix it. Thanks
Flags: needinfo?(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.
Flags: needinfo?(bzbarsky)
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.
Flags: needinfo?(bzbarsky)
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.
Flags: needinfo?(ydelendik)
Depends on: 1217047
Flags: needinfo?(bzbarsky)
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?
Nathan, see comment 10?
Flags: needinfo?(mbebenita) → needinfo?(nfroyd)
(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.
Flags: needinfo?(nfroyd)
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?
Flags: needinfo?(alice0775)
(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.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(alice0775)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.