999443 - IE Tab Plug-in is not automatically activated

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Paul Silaghi, QA [:pauly]]

STR:
1. Install https://addons.mozilla.org/addon/ie-tab-2-ff-36/
2. Restart FF
3. Open Addons Manager/Plugins

Actual result: IE Tab Plug-in is set to 'Ask to Activate'
Expected result: IE Tab Plug-in is set to 'Always Activate'

Path: C:\Users\paul.silaghi\AppData\Roaming\Mozilla\Firefox\Profiles\jhkta1cy.21\extensions\{1BC9BA34-1EED-42ca-A505-6D2F1A935BBB}\plugins\npietab2.dll

Comment 1

[Paul Silaghi, QA [:pauly]]

31.0a1 (2014-04-21), Win 7 x64

Comment 2

[Benjamin Smedberg]

Attachment: plugin-extension-test

Comment 3

[Benjamin Smedberg]

Attachment: plugin-extension-fix

Comment 4

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8410409 [details] [diff] [review]
plugin-extension-test

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

r+ with comments addressed.

::: dom/plugins/test/unit/head_plugins.js
@@ +12,5 @@
>  const gIsLinux = ("@mozilla.org/gnome-gconf-service;1" in Cc) ||
>    ("@mozilla.org/gio-service;1" in Cc);
>  const gDirSvc = Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties);
>  
> +const kPluginNames = [

k-prefixing for constants seems unusual in test js - use upper- or plain mixed-case without prefix?

@@ +29,5 @@
>    var pluginEnum = gDirSvc.get("APluginsDL", Ci.nsISimpleEnumerator);
>    while (pluginEnum.hasMoreElements()) {
>      let dir = pluginEnum.getNext().QueryInterface(Ci.nsILocalFile);
> +    let names = secondplugin ? kSecondNames : kPluginNames;
> +    for (let name of names) {

Can't this just use |get_platform_specific_plugin_name()| plus handling for the second plugin?

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8410411 [details] [diff] [review]
plugin-extension-fix

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

Ouch, that was quite an oversight.

::: dom/plugins/base/nsPluginHost.cpp
@@ +1633,4 @@
>  {
> +  for (uint32_t i = 0; i < extensionDirs.Length(); ++i) {
> +    bool contains;
> +    if (NS_FAILED(extensionDirs[i]->Contains(pluginFile, true, &contains)) || !contains) {

So we now automatically activate plugins no matter where they are in extension dirs, not just <extension-dir>/plugins/.
So far i can't think of a problem with this though.

@@ +1660,5 @@
>    }
>  
> +  bool more;
> +  nsCOMPtr<nsISupports> next;
> +  nsCOMPtr<nsIFile> file;

next & file don't need to be in this scope, can just move them where they are actually needed.

Comment 6

[Benjamin Smedberg]

code https://hg.mozilla.org/integration/fx-team/rev/6892fe04273f
test https://hg.mozilla.org/integration/fx-team/rev/7ddc0333be78

Comment 7

[Benjamin Smedberg]

Comment on attachment 8410411 [details] [diff] [review]
plugin-extension-fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 982101
User impact if declined: feature bug 982101 doesn't work, blocks bug 992995
Testing completed (on m-c, etc.): just landed, automated tests and we'll have manual QA verify also
Risk to taking this patch (and alternatives if risky): fairly low risk
String or IDL/UUID changes made by this patch: None

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6892fe04273f
https://hg.mozilla.org/mozilla-central/rev/7ddc0333be78

Comment 10

[Marco Mucci [:MarcoM]]

Hi Paul, confirming if this resolved bug can be verified before the end of the desktop iteration on Monday April 28?

Comment 11

[Paul Silaghi, QA [:pauly]]

Confirmed

Comment 12

[Paul Silaghi, QA [:pauly]]

IE Tab Plug-in is now set to 'Always Activate' after installation.
Verified fixed 31.0a1 (2014-04-24), Win 7 x64.
The plugin is N/A for Mac and Linux.

Comment 13

[Paul Silaghi, QA [:pauly]]

I noticed 2 other issues with this addon (both reproducible on builds before landing bug 982101):

1. With the plugin 'Always activate', right click/open in IE Tab on a link, block the plugin from the doorhanger and then reload => plugin crash https://crash-stats.mozilla.com/report/index/d0ef86f6-577a-4511-a538-d7f4b2140425

2. Setting IT Tab to 'Ask to Activate' doesn't really block the plugin (right click/open in IE Tab on a link works just fine). Allowing from the doorhanger and then re-blocking finally blocks the plugin (the CTP overlay is displayed)

Let me know is these worth to be filed separately.

Comment 14

[Benjamin Smedberg]

No, that's part of the addon and not something to track here.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/622ae811fe06
https://hg.mozilla.org/releases/mozilla-aurora/rev/7940ae66d505

Comment 16

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 30 Aurora
Build Id:20140428004000
OS: Win7 x64