All users were logged out of Bugzilla on October 13th, 2018

Firefox Nighty 24.0a1 opens blank page instead of calling the third-party PDF reader plugin

VERIFIED FIXED in Firefox 24

Status

()

P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: ananuti, Assigned: jst)

Tracking

({regression})

24 Branch
Firefox 25
x86
Windows 7
regression
Points:
---
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(firefox23 unaffected, firefox24+ verified, firefox25+ verified)

Details

(Whiteboard: [pdfjs-c-integration])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
STR:
1. start Nightly with virgin profile
2. enable CTP plugin, flip plugins.click_to_play to true and restart
3. set acrobat plugin to "Ask to Activate" 
4. set Portable Document Format (PDF) (in Options::Application) to Use Adobe Acrobat (in Nightly) and restart


m-c:
last good: mozilla-central changeset 6edffc15e2dc
first bad: mozilla-central changeset 198e38876f7e Merge m-i and m-c 


m-i:
last good: mozilla-inbound changeset cd7ca5763eac
first bad: mozilla-inbound changeset ec45e0246b83

user:        Johnny Stenback <jst@mozilla.com>
date:        Wed Mar 27 22:19:41 2013 -0700
files:       browser/base/content/pageinfo/permissions.js dom/plugins/base/nsIPluginTag.idl dom/plugins/base/nsPluginTags.cpp dom/plugins/test/mochitest/test_refresh_navigator_plugins.html toolkit/mozapps/extensions/PluginProvider.jsm
description:
Bug 855613. Stop exposing nsIDOMMimeType in nsIPluginTag. r=joshmoz@gmail.com
Priority: -- → P3
Is this with the most recent or a specific version of the Acrobat plugin?
(Reporter)

Comment 2

5 years ago
Adobe Acrobat 11.0.3.37 (I haven't tested earlier versions.)
(Reporter)

Updated

5 years ago
Keywords: regression

Updated

5 years ago
status-firefox23: --- → unaffected
status-firefox24: --- → affected
status-firefox25: --- → affected
tracking-firefox24: --- → ?
tracking-firefox25: --- → ?
Version: Trunk → 24 Branch

Comment 3

5 years ago
Almost same Bug 855666 again  again  again  again,
Automation test should be provided!

Comment 4

5 years ago
>Almost same Bug 855666 again  again  again  again,
>Automation test should be provided!

Yes, there should be an Automation test.

But taking it in account that this bug appears again and again, we should rethink whether the fix for Bug 738967, which I think is the root cause of this regression, is really necessary at all.
In my opinion, to support for embedded PDF with pdf.js is not essential at all.

Comment 5

5 years ago
We're still not shipping a visible pref for this, correct Georg? Is step 2 necessary to repro? If yes to both, this won't track.

Comment 6

5 years ago
(In reply to Alex Keybl [:akeybl] from comment #5)
> We're still not shipping a visible pref for this, correct Georg? Is step 2
> necessary to repro? If yes to both, this won't track.

Not need to change preferences(step1-3) in about:config.
It is necessary "step4" only.

Comment 7

5 years ago
What is the repro URL?

Comment 8

5 years ago
To answer akeybl, there is no visible pref in 24 (although I'm thinking about asking for uplift if I can get bug 888510 in time). In 25 we flipped the default so that click-to-activate is visible in the addon manager.
Created attachment 771057 [details] [diff] [review]
Fix.

This fixes 'a' bug in PDF.js, it might be this one, or it might be something that manifests itself in different ways. Found by code inspection here. This PDF.js change came into m-c after my changes landed that changed what nsIPluginTag.getMimeTypes() returns, and this needs to be fixed upstream in PDF.js as well, however that process is managed. Brendan, please roll this up where needed, whether it fixes this bug or not (and please help test too, I don't have a testcase for this bug readily available).
Attachment #771057 - Flags: review?(bdahl)
(Reporter)

Comment 10

5 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> What is the repro URL?

http://people.mozilla.com/~lco/CtP/130415%20CtP%20design%20spec.pdf

Comment 11

5 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> What is the repro URL?
Actually all pdfs are reproducible.

How to reproduce:
Just set Portable Document Format (PDF) (in Options::Application) to Use Adobe Acrobat (in Nightly) and access to any PDF in the web.
(Reporter)

Comment 12

5 years ago
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #9)
> Created attachment 771057 [details] [diff] [review]
> Untested but possible fix.

I can verify that this patch build does indeed fix the problem.
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ananuti@gmail.com-92ab62a604f3/

Comment 13

5 years ago
> I can verify that this patch build does indeed fix the problem.
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ananuti@gmail.com-
> 92ab62a604f3/

I also verified that this patch worked.
But I wonder if Aurora(Firefox 24) will be updated or not.
Comment on attachment 771057 [details] [diff] [review]
Fix.

Thanks for testing the fix! I'll leave it up to bdahl etc to decide whether this needs to be uplifted. But I will say that this is about as safe and as low impact as bug fixes get.
Attachment #771057 - Attachment description: Untested but possible fix. → Fix.
Comment on attachment 771057 [details] [diff] [review]
Fix.

>-        return mimeType.type === PDF_CONTENT_TYPE;
>+        return mimeType === PDF_CONTENT_TYPE;

For upstream, it should be |(mimeType.type || mimeType) === PDF_CONTENT_TYPE|. We need to support older versions. (It would be sufficient for Aurora internal viewer.)
Component: Plug-ins → PDF Viewer
Priority: P3 → --
Product: Core → Firefox
(In reply to Masatoshi Kimura [:emk] from comment #15)
> Comment on attachment 771057 [details] [diff] [review]
> Fix.
> 
> >-        return mimeType.type === PDF_CONTENT_TYPE;
> >+        return mimeType === PDF_CONTENT_TYPE;
> 
> For upstream, it should be |(mimeType.type || mimeType) ===
> PDF_CONTENT_TYPE|. We need to support older versions. (It would be
> sufficient for Aurora internal viewer.)

Pdfjs.jsm isn't used for the extension so the current fix should be sufficient.
Comment on attachment 771057 [details] [diff] [review]
Fix.

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

Upstream PR:
https://github.com/mozilla/pdf.js/pull/3450

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879161
User impact if declined: for pdf plugins if click to play is enabled the user will not be able to use third party plugins
Testing completed (on m-c, etc.): tested locally, we could land on MC first then aurora if wanted
Risk to taking this patch (and alternatives if risky): very low, simple check
String or IDL/UUID changes made by this patch: none
Attachment #771057 - Flags: review?(bdahl)
Attachment #771057 - Flags: review+
Attachment #771057 - Flags: approval-mozilla-aurora?

Updated

5 years ago
tracking-firefox24: ? → +
tracking-firefox25: ? → +

Updated

5 years ago
Priority: -- → P1
Whiteboard: [pdfjs-c-integration]
(In reply to Brendan Dahl from comment #17)
> Comment on attachment 771057 [details] [diff] [review]
> Fix.
> 
> Review of attachment 771057 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Upstream PR:
> https://github.com/mozilla/pdf.js/pull/3450
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): bug 879161
> User impact if declined: for pdf plugins if click to play is enabled the
> user will not be able to use third party plugins
> Testing completed (on m-c, etc.): tested locally, we could land on MC first
> then aurora if wanted
> Risk to taking this patch (and alternatives if risky): very low, simple check
> String or IDL/UUID changes made by this patch: none

Yep can we please go ahead with m-c landing first.Also adding qawanted,verifyme once this lands.
Keywords: qawanted, verifyme

Updated

5 years ago
Keywords: checkin-needed
Mihaela, please check the latest mozilla-central build as soon as this lands.
QA Contact: mihaela.velimiroviciu
https://hg.mozilla.org/mozilla-central/rev/af805ebcb52e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox25: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
(Reporter)

Comment 22

5 years ago
Mozilla/5.0 (Windows NT 6.1; rv:25.0) Gecko/20130712 Firefox/25.0

verified fixed nightly 25.0a1 buildid 20130712030203
status-firefox25: fixed → verified

Updated

5 years ago
Attachment #771057 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/c06bdff31cff
status-firefox24: affected → fixed
Keywords: checkin-needed
(Reporter)

Comment 24

5 years ago
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20130717 Firefox/24.0

Verified fixed Aurora 24.0a2 buildID 20130717004002
Status: RESOLVED → VERIFIED
status-firefox24: fixed → verified
Thanks for your help Ekanan.
Keywords: verifyme

Comment 26

5 years ago
Given that this has recurred twice, it seems that we need a manual test for it (it's basically impossible to test in the automated suite without installing a PDF plugin on the build slaves, which is inadvisable).
Flags: needinfo?(mihaela.velimiroviciu)
I added a manual testcase for this in Moztrap: https://moztrap.mozilla.org/manage/cases/?filter-id=9085
Flags: needinfo?(mihaela.velimiroviciu)
Keywords: qawanted
Benjamin and Mihaela, in the future please use the in-moztrap flag for manual test creation requests. Thanks.
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.