Closed Bug 738674 Opened 12 years ago Closed 12 years ago

Addon blocker shouldn't block built-in PDF viewer

Categories

(Firefox :: PDF Viewer, defect)

Product:
Firefox
Component:
PDF Viewer
Version:
14 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 15
Tracking Flags:
Tracking Status
firefox15 + fixed

People

(Reporter: lmandel, Unassigned)

References

Details

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?
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.
What about checking a certificate?
How does Test Pilot get installed?
(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?
(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.
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.
(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?
(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.
Another side effect of being considered as a third party add-on is that it's disabled with a new profile.
Blocks: 714712
No longer depends on: 714712
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?
(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.
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.
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.
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?
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?
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.
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.
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).
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.
Depends on: 740795
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.
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,
(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.
(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!!!!
(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.
I'll repeat comment 14: why does pdf.js have to be an addon at all?
(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.
(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
(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.
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?
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).
Isn't JBIG2 protected by patents?
(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.
(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.
(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.
(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.
(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.
(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.
Blocks: 748923
Summary: pdf.js installation prompt is blocked by add-on blocking → Addon blocker shouldn't block built-in PDF viewer
(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.
(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
Blocks: 752676
this was fixed by bug 740795
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
You need to log in before you can comment on or make changes to this bug.