Last Comment Bug 703815 - Click to activate message is displayed even if plugins are disabled
: Click to activate message is displayed even if plugins are disabled
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: mozilla11
Assigned To: Brad Lassey [:blassey] (use needinfo?)
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on:
Blocks: 704612
  Show dependency treegraph
 
Reported: 2011-11-18 18:20 PST by Brad Lassey [:blassey] (use needinfo?)
Modified: 2012-02-01 13:58 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
patch (1.72 KB, patch)
2011-11-18 18:20 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch (1.92 KB, patch)
2011-11-18 18:21 PST, Brad Lassey [:blassey] (use needinfo?)
doug.turner: review-
Details | Diff | Splinter Review
patch (1.67 KB, patch)
2011-11-19 13:54 PST, Brad Lassey [:blassey] (use needinfo?)
doug.turner: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Brad Lassey [:blassey] (use needinfo?) 2011-11-18 18:20:01 PST
Created attachment 575613 [details] [diff] [review]
patch
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2011-11-18 18:21:43 PST
Created attachment 575614 [details] [diff] [review]
patch
Comment 2 Doug Turner (:dougt) 2011-11-18 22:32:09 PST
Comment on attachment 575614 [details] [diff] [review]
patch

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

::: content/base/src/nsObjectLoadingContent.cpp
@@ +1965,5 @@
>  #ifdef ANDROID
> +   nsCOMPtr<nsIPrefBranch> prefService = do_GetService(NS_PREFSERVICE_CONTRACTID);
> +  bool pluginsDisabled = false;
> +  if (prefService) {
> +    nsresult rv = prefService->GetBoolPref("plugin.disable", &pluginsDisabled);

use Preferences::GetBool()

@@ +1969,5 @@
> +    nsresult rv = prefService->GetBoolPref("plugin.disable", &pluginsDisabled);
> +    if (NS_FAILED(rv))
> +      pluginsDisabled = false;
> +  }
> +  if (!pluginsDisabled && XRE_GetProcessType() == GeckoProcessType_Content)

Why do we need to check for the content process?
Comment 3 Brad Lassey [:blassey] (use needinfo?) 2011-11-19 13:45:55 PST
(In reply to Doug Turner (:dougt) from comment #2)
> Comment on attachment 575614 [details] [diff] [review] [diff] [details] [review]
> @@ +1969,5 @@
> > +    nsresult rv = prefService->GetBoolPref("plugin.disable", &pluginsDisabled);
> > +    if (NS_FAILED(rv))
> > +      pluginsDisabled = false;
> > +  }
> > +  if (!pluginsDisabled && XRE_GetProcessType() == GeckoProcessType_Content)
> 
> Why do we need to check for the content process?

Because we only use this version of click to play in the content process
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2011-11-19 13:54:10 PST
Created attachment 575696 [details] [diff] [review]
patch
Comment 5 Doug Turner (:dougt) 2011-11-19 18:32:34 PST
> Because we only use this version of click to play in the content process
Don't we want to click to play in the native client.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2011-11-19 19:17:32 PST
(In reply to Doug Turner (:dougt) from comment #5)
> > Because we only use this version of click to play in the content process
> Don't we want to click to play in the native client.

yes, that is being implemented in bug 549697. This is a bug in our tab-restarting version of click to play for e10s fennec which must be resolved for 10 so we can disable plugins and not look awful.
Comment 7 Doug Turner (:dougt) 2011-11-19 20:03:09 PST
Comment on attachment 575696 [details] [diff] [review]
patch

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

r+ with a comment.  Please land this on m-c.

::: content/base/src/nsObjectLoadingContent.cpp
@@ +1963,4 @@
>  nsObjectLoadingContent::GetPluginDisabledState(const nsCString& aContentType)
>  {
>  #ifdef ANDROID
> +  if (!mozilla::Preferences::GetBool("plugin.disable", false) && 

Please add a comment here to say something like:

// If plugin.disable is false or not set,  we want to show click-to-play.
Comment 8 Brad Lassey [:blassey] (use needinfo?) 2011-11-19 20:10:58 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/6afb61406ead
Comment 9 Ed Morley [:emorley] 2011-11-20 14:20:52 PST
https://hg.mozilla.org/mozilla-central/rev/6afb61406ead
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2011-11-23 11:11:43 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/f0b2f0baf91d

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