Last Comment Bug 738674 - Addon blocker shouldn't block built-in PDF viewer
: Addon blocker shouldn't block built-in PDF viewer
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: PDF Viewer (show other bugs)
: 14 Branch
: All All
: -- normal (vote)
: Firefox 15
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 748931 (view as bug list)
Depends on: 740795
Blocks: 714712 748923 752676
  Show dependency treegraph
 
Reported: 2012-03-23 09:15 PDT by Lawrence Mandel [:lmandel] (use needinfo)
Modified: 2012-06-04 14:59 PDT (History)
38 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments

Description Lawrence Mandel [:lmandel] (use needinfo) 2012-03-23 09:15:01 PDT
As Dave Townsend suspected in bug 714712 comment 101, installation of pdf.js is blocked by add-on blocking. The prompt that shows "Another program on your computer would like to modify Nightly with the following add-on:" is confusing as well as it is Firefox that is installing the add-on. Can we either avoid the add-on block for our own add-on (preferable) or include a better message stating that it is Mozilla Firefox that wants to install the add-on?
Comment 1 Dave Townsend [:mossop] 2012-03-23 09:52:41 PDT
The add-on blocking code has no way to distinguish between add-ons shipped with the application and add-ons injected by a third party right now (except for the default theme) and I can't think of an easy way we could add that that third-parties wouldn't just abuse.
Comment 2 Masatoshi Kimura [:emk] 2012-03-23 10:03:51 PDT
What about checking a certificate?
Comment 3 Ed Lee :Mardak 2012-03-23 10:42:19 PDT
How does Test Pilot get installed?
Comment 4 Siddharth Agarwal [:sid0] (inactive) 2012-03-23 13:22:04 PDT
(In reply to Dave Townsend (:Mossop) from comment #1)
> The add-on blocking code has no way to distinguish between add-ons shipped
> with the application and add-ons injected by a third party right now (except
> for the default theme) and I can't think of an easy way we could add that
> that third-parties wouldn't just abuse.

Don't we have a way to sign extensions?
Comment 5 Dave Townsend [:mossop] 2012-03-23 13:26:05 PDT
(In reply to Edward Lee :Mardak from comment #3)
> How does Test Pilot get installed?

As a distribution extension, this has some different behaviours, notably users can uninstall the add-on and wouldn't have a way to get it back, and Nightly testers wouldn't even see the add-on at first, nor would it be easy to update it in nightly builds since we only check distribution add-ons when the Firefox version changes.

(In reply to Siddharth Agarwal [:sid0] from comment #4)
> (In reply to Dave Townsend (:Mossop) from comment #1)
> > The add-on blocking code has no way to distinguish between add-ons shipped
> > with the application and add-ons injected by a third party right now (except
> > for the default theme) and I can't think of an easy way we could add that
> > that third-parties wouldn't just abuse.
> 
> Don't we have a way to sign extensions?

This is something I hadn't considered and would be possible, though we'd have to figure out some way to sign the XPI as part of the build process which sounds like a tricky challenge for releng.
Comment 6 Siddharth Agarwal [:sid0] (inactive) 2012-03-23 13:28:55 PDT
Is it that tricky, though? We already sign exes and dlls for nightly builds. I think we recognize object signing certificates for extensions too, so we could even probably use the same cert.
Comment 7 neil@parkwaycc.co.uk 2012-03-23 13:31:37 PDT
(In reply to Dave Townsend from comment #5)
> As a distribution extension, this has some different behaviours, notably
> users can uninstall the add-on and wouldn't have a way to get it back, and
> Nightly testers wouldn't even see the add-on at first, nor would it be easy
> to update it in nightly builds since we only check distribution add-ons when
> the Firefox version changes.
Why not just update it on AMO?
Comment 8 Dave Townsend [:mossop] 2012-03-23 13:32:57 PDT
(In reply to Siddharth Agarwal [:sid0] from comment #6)
> Is it that tricky, though? We already sign exes and dlls for nightly builds.
> I think we recognize object signing certificates for extensions too, so we
> could even probably use the same cert.

I have no idea, we should ask someone from releng.
Comment 9 Scoobidiver (away) 2012-03-24 10:37:42 PDT
Another side effect of being considered as a third party add-on is that it's disabled with a new profile.
Comment 10 :aceman 2012-03-28 04:10:05 PDT
Why do you want to hide this extension and not show the blocking page to the users?
What if they already have a pdf extension/plugin or do not want any handling?
Comment 11 Scoobidiver (away) 2012-03-28 04:24:34 PDT
(In reply to :aceman from comment #10)
> What if they already have a pdf extension/plugin or do not want any handling?
See bug 738955.
Comment 12 :aceman 2012-03-28 04:49:37 PDT
Even worse:)
So if Firefox installs the pdf.js secretly it will disable the user's preferred plugin. I think pdf.js can be installed by default, but if a plugin is existing it should take priority.
Comment 13 Marco Castelluccio [:marco] 2012-03-28 05:40:08 PDT
It isn't "installed secretly". It's "installed" like every other new Firefox feature. And like most new features, it can be disabled.
.
Anyway consider that most users have a plugin to read PDFs files, but they didn't choose it. If we give priority to existing plugins, no one will use PDF Viewer.
Comment 14 Mike Hommey [:glandium] 2012-03-28 06:11:43 PDT
Speaking of other new Firefox feature, what makes pdf.js special in that we ship it as a bundled extension? Wouldn't incorporating it like other features solve this bug, and many other related issues?
Comment 15 :aceman 2012-03-28 06:13:01 PDT
Why do we want them to use our PDF viewer, when that already have one? What do we gain?

So you assume they didn't choose their plugin because some other SW bundle installed it secretly (like Adobe reader). And now you want to remedy this by doing the same and secretly install a different addon? I think it is not correct.

Even if the user didn't install the plugin, he is now used to be able to open .pdfs fine. He will now notice something has changed, that pdfs still open but differently and with reduced functionality. Will this open a ton of reports?
Comment 16 Lawrence Mandel [:lmandel] (use needinfo) 2012-03-28 07:27:53 PDT
Comments 12-15 are really a question of "should we bundle pdf.js in Firefox". While certainly a valid question I would suggest that this is really a topic for another bug.

My request in this bug is to improve the installation experience of pdf.js.
Comment 17 Marco Castelluccio [:marco] 2012-03-28 07:32:06 PDT
PDF Viewer is more secure than an external binary plugin and is better integrated in the browser.
If there wasn't any gain in using pdf.js, Mozilla wouldn't develop it...
https://wiki.mozilla.org/Show_PDF_inline

We don't install "secretly" the PDF Viewer. It's a Firefox feature, like every other feature. The fact that it's an addon doesn't make any difference.

However, as Lawrence says, this is the wrong place to discuss this.
Comment 18 :aceman 2012-03-28 10:46:06 PDT
I am for bundling the addon with Firefox, do not misinterpret me.
I just do not like it that it silently replaces existing user selected plugin (together with bug 738955).
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-28 10:51:08 PDT
Most PDF viewers are not "user selected", they're "user's only option". Many of them are out of date and insecure, and can cause crashes or other issues.

The people who care to carefully select their PDF viewer can still do it (by disabling Firefox's PDF viewer). For the vast majority of our users who don't, and just want PDFs to work and be secure, we want them to get the benefits of the built-in PDF viewer.

Let's please stop discussing this in this bug - if you'd like to discuss this further feel free to file a new bug, or better yet start a thread in dev.apps.firefox.
Comment 20 Jesper Hansen 2012-03-30 07:35:16 PDT
Enabling pdf.js by default and disabling something like adobe's pdf reader might actually cause a lawsuit since it would cause adobe to lose marked share.
Comment 21 Johnathan Nightingale [:johnath] 2012-03-30 07:38:42 PDT
Oh I *guarantee* that this bug is not the place for legal speculation.

Gavin's right in comment 19 - this bug is about the broken way our new PDF-reading-in-the-browser feature behaves on first run. If you want to argue about the feature as a whole, or the global sociolegal context in which it exists, take it to the newsgroups, please,
Comment 22 Dave Townsend [:mossop] 2012-04-11 09:15:48 PDT
(In reply to Dave Townsend (:Mossop) from comment #5)
> (In reply to Siddharth Agarwal [:sid0] from comment #4)
> > (In reply to Dave Townsend (:Mossop) from comment #1)
> > > The add-on blocking code has no way to distinguish between add-ons shipped
> > > with the application and add-ons injected by a third party right now (except
> > > for the default theme) and I can't think of an easy way we could add that
> > > that third-parties wouldn't just abuse.
> > 
> > Don't we have a way to sign extensions?
> 
> This is something I hadn't considered and would be possible, though we'd
> have to figure out some way to sign the XPI as part of the build process
> which sounds like a tricky challenge for releng.

I just realised that I'm not sure whether this is going to help us much. The normal way I'd expect to do this is to embed data about the signing certificate into preferences that we used to verify the XPI when it is detected. This allows other applications (Seamonkey f.e.) and other distributions (Ubuntu f.e.) to easily use their own signing key if they wanted. But it would also make it trivial for any third party dropping their extension into the firefox directory to sign it and put their own signing key into the prefs to accept it.

The alternative is to hard-code the acceptable keys into the add-ons manager code, but that means that other apps and distributions would effectively have to fork the add-ons manager to ship their own stuff which seems like a horrible state to be in.
Comment 23 Justin Wood (:Callek) 2012-04-11 10:21:26 PDT
(In reply to neil@parkwaycc.co.uk from comment #7)
> (In reply to Dave Townsend from comment #5)
> > As a distribution extension, this has some different behaviours, notably
> > users can uninstall the add-on and wouldn't have a way to get it back, and
> > Nightly testers wouldn't even see the add-on at first, nor would it be easy
> > to update it in nightly builds since we only check distribution add-ons when
> > the Firefox version changes.
> Why not just update it on AMO?

This is what SeaMonkey does... after having discussed with you (Mossop) among others when we were migrating to toolkit-based, and also once the new addon manager was re-written.

* Basically we now ship all our extensions in distribution/*
* All the extensions are also available on AMO if a user uninstalls one and wants it back
* All the extensions are updateable by the app itself when version numbers change (updates!)
* All the extensions are updateable by AMOs update system
* Avoids the addon-block methodology.
* Allows the user to disable the addon easily (per profile)
* Allows AMO updates independent of Application Updates!!!!
Comment 24 Blair McBride [:Unfocused] (UNAVAILABLE) 2012-04-12 07:00:01 PDT
(In reply to Dave Townsend (:Mossop) from comment #22)
> The alternative is to hard-code the acceptable keys into the add-ons manager
> code, but that means that other apps and distributions would effectively
> have to fork the add-ons manager to ship their own stuff which seems like a
> horrible state to be in.

There is another alternative: Use a separate configuration file in omni.jar, beside greprefs.js. 

But we really do need to figure out exactly what behaviour we want here, as all that may be unnecessary.
Comment 25 Mike Hommey [:glandium] 2012-04-12 07:06:04 PDT
I'll repeat comment 14: why does pdf.js have to be an addon at all?
Comment 26 Dave Townsend [:mossop] 2012-04-12 09:29:35 PDT
(In reply to Mike Hommey [:glandium] from comment #25)
> I'll repeat comment 14: why does pdf.js have to be an addon at all?

This is a question I'm waiting to hear back on from the product drivers, I'm hoping to hear back soon.
Comment 27 Bill Walker [:bwalker] [@wfwalker] 2012-04-12 12:15:03 PDT
(In reply to Mike Hommey [:glandium] from comment #25)
> I'll repeat comment 14: why does pdf.js have to be an addon at all?

PDF.js doesn't _have_ to be an Add-on at all. But it does seem to be a convenient, modular, and secure way to deliver this functionality, and so far we have not encountered any compelling reason why it shouldn't be an Add-on.

See also https://wiki.mozilla.org/Firefox/BundlingAdd-ons
Comment 28 Dave Townsend [:mossop] 2012-04-12 12:24:55 PDT
(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #27)
> (In reply to Mike Hommey [:glandium] from comment #25)
> > I'll repeat comment 14: why does pdf.js have to be an addon at all?
> 
> PDF.js doesn't _have_ to be an Add-on at all. But it does seem to be a
> convenient, modular, and secure way to deliver this functionality, and so
> far we have not encountered any compelling reason why it shouldn't be an
> Add-on.

We can ship it with Firefox with basically no code change to pdf.js but without it appearing as an add-on and getting it blocked by the add-on blocking code. I'm working on testing a patch that does that now.

Whether it appears as an add-on in Firefox or not makes no difference to the modularity or security. I don't think it makes a meaningful difference to the convenience either.
Comment 29 Dietrich Ayala (:dietrich) 2012-04-12 13:16:22 PDT
To clarify Dave's comment: There's a way to keep PDF.js a bundled add-on but have it *not* appear in the Add-ons Manager, and which removes the add-on blocking problem.

His patch implements that, and he's going to detail that approach on the bundling wiki.

With this approach, users cannot disable the functionality via the Add-ons Manager. Bill, can you comment on whether this is a problem or not?
Comment 30 Josh Matthews [:jdm] 2012-04-12 13:18:49 PDT
That sounds like a problem to me, given that there is a nontrivial subset of PDFs that pdf.js displays as complete garbage right now (ie. it is lacking a JBIG2 decoder, and a surprising number of PDFs are completely comprised of JBIG2-encoded images).
Comment 31 Marco Castelluccio [:marco] 2012-04-12 13:21:18 PDT
Isn't JBIG2 protected by patents?
Comment 32 Dave Townsend [:mossop] 2012-04-12 13:33:32 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #29)
> With this approach, users cannot disable the functionality via the Add-ons
> Manager. Bill, can you comment on whether this is a problem or not?

It would be pretty simple to add a checkbox turning it on/off into the preferences of course, we even talked about adding it to the application prefs.

Given that right now pdf.js is automatically ignored if adobe reader or other pdf plugins are present, turning it off means you just end up with a download dialog.
Comment 33 Bill Walker [:bwalker] [@wfwalker] 2012-04-12 14:01:23 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #29)
> To clarify Dave's comment: There's a way to keep PDF.js a bundled add-on but
> have it *not* appear in the Add-ons Manager, and which removes the add-on
> blocking problem.
> 
> His patch implements that, and he's going to detail that approach on the
> bundling wiki.
> 
> With this approach, users cannot disable the functionality via the Add-ons
> Manager. Bill, can you comment on whether this is a problem or not?

Dietrich, thanks for the clarification, I completely misunderstood that.
Comment 34 Bill Walker [:bwalker] [@wfwalker] 2012-04-12 14:05:25 PDT
(In reply to Dave Townsend (:Mossop) from comment #32)
> (In reply to Dietrich Ayala (:dietrich) from comment #29)
> > With this approach, users cannot disable the functionality via the Add-ons
> > Manager. Bill, can you comment on whether this is a problem or not?
> 
> It would be pretty simple to add a checkbox turning it on/off into the
> preferences of course, we even talked about adding it to the application
> prefs.
> 
> Given that right now pdf.js is automatically ignored if adobe reader or
> other pdf plugins are present, turning it off means you just end up with a
> download dialog.

This sounds right -- there's no need for PDF.js to appear in the Add-on Manager (re: comment 29), but I think users do need a straightforward way to say which PDF viewer they want to use.
Comment 35 Scoobidiver (away) 2012-04-12 14:17:00 PDT
(In reply to Dave Townsend (:Mossop) from comment #28)
> Whether it appears as an add-on in Firefox or not makes no difference to the
> modularity or security.
There is a difference because you can make the PDF.js code evolves between two Firefox release (hotfix) if it's a visible add-on while you can't for traceability reasons if it is invisible.

(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #34)
> I think users do need a straightforward way to say which PDF viewer they want
> to use.
See bug 738955.
Comment 36 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-04-12 14:19:02 PDT
(In reply to Scoobidiver from comment #35)
> There is a difference because you can make the PDF.js code evolves between
> two Firefox release (hotfix) if it's a visible add-on while you can't for
> traceability reasons if it is invisible.

We're not going to be updating pdf.js out-of-band from the normal Firefox updates, as far as I know.
Comment 37 Dave Townsend [:mossop] 2012-04-12 14:40:08 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #36)
> (In reply to Scoobidiver from comment #35)
> > There is a difference because you can make the PDF.js code evolves between
> > two Firefox release (hotfix) if it's a visible add-on while you can't for
> > traceability reasons if it is invisible.
> 
> We're not going to be updating pdf.js out-of-band from the normal Firefox
> updates, as far as I know.

Right, the packaging mechanism we use currently doesn't even allow that automatically (nor is it wanted to my understanding). If you mean manually installing updated versions then that is still possible regardless of how we ship pdf.js, add-ons can replace parts of Firefox, it just might require a little more work in some cases. In pdf.js's case it should just work automatically I believe.

I just chatted with Asa and he actually prefers that pdf.js not be listed in the add-ons manager so I'm going to put the finishing touches to my patch and then run it through try.
Comment 38 Artur Adib 2012-04-25 13:54:27 PDT
*** Bug 748931 has been marked as a duplicate of this bug. ***
Comment 39 Mark Hammond [:markh] 2012-04-30 19:48:28 PDT
(In reply to Dave Townsend (:Mossop) from comment #37)
> I'm going to put the finishing touches to my patch
> and then run it through try.

Is this patch available somewhere?  I'd like to try it out on our experiment of bundling a "social" addon with firefox.
Comment 40 Dave Townsend [:mossop] 2012-04-30 21:19:35 PDT
(In reply to Mark Hammond (:markh) from comment #39)
> (In reply to Dave Townsend (:Mossop) from comment #37)
> > I'm going to put the finishing touches to my patch
> > and then run it through try.
> 
> Is this patch available somewhere?  I'd like to try it out on our experiment
> of bundling a "social" addon with firefox.

See bug 740795
Comment 41 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-04 14:58:50 PDT
this was fixed by bug 740795

Note You need to log in before you can comment on or make changes to this bug.