1133003 - When users experience a Flash plugin hang, turn off Flash protected mode

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[Benjamin Smedberg]

Currently we have a hook which disabled Flash protected mode, but no UI to expose this to users.

Since turning protected mode off for all users may be a security risk we are unwilling to bear, we are considering offering users a way to opt into this option if/when they experience a Flash hang.

This is complicated because most users are incapable of making an informed choice to trade off security versus functionality, and the security benefits and risks are not easily measurable.

Comment 1

[Geoff Piper]

Adding myself to this but I believe, once the overall plan is adopted / approved, the next action item here is to develop the actual copy that will make up the notice (I am not sure who should own that whether Eric, Erica or Mary Ellen or a combination), implemented by Madhava's team (I assume this is a door hanger or pop up notice of some kind), and in my view perhaps we add a Learn More link to that notice which maps to a Wiki, SUMO article, or Ben perhaps a Blog post explaining all of the ins and out.

Comment 2

[Benjamin Smedberg]

UX team typically writes the in-product notice. They'll decide whether to include a learn more link, which would go to SUMO.

Comment 3

[Benjamin Smedberg]

Attachment: 1133003-partA

Comment 4

[Benjamin Smedberg]

Attachment: 2015-02-19_flash-flipui.png

Comment 5

[Benjamin Smedberg]

Attachment: 1133003-partB

Haven't gotten final UX review on this, but we're close to the 38 wire so I'm going to put up what I've got for code review. I like simple!

Comment 6

[Madhava Enros [:madhava]]

Comment on attachment 8566805 [details]
2015-02-19_flash-flipui.png

Firefox has changed some Adobe Flash settings to improve performance.

Comment 7

[Geoff Piper]

Where doe the Learn More button go and what is on the Learn More link page? Who is authoring that content?

Comment 8

[Madhava Enros [:madhava]]

Active voice for Chad:

Firefox changed some Adobe Flash settings to improve performance.             [Learn More]

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Madhava Enros [:madhava] from comment #8)
> Active voice for Chad:
> 
> Firefox changed some Adobe Flash settings to improve performance.           
> [Learn More]

"to improve performance" or "to improve reliability"? I think reliability makes more sense here since it will be (hopefully) less crashy after the change. But maybe performance and reliability are synonymous here.

Comment 10

[Benjamin Smedberg]

I'd like to stick with "performance" both because I think it covers the whole gamut of how well software performs, but also because the things we're optimizing here is hangs, which are primarily perceived by users as Firefox being slow.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8566826 [details] [diff] [review]
1133003-partB

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

r=me with the following fixed

::: browser/components/nsBrowserGlue.js
@@ +2074,5 @@
> +      return;
> +    }
> +    let productName = Services.strings
> +      .createBundle("chrome://branding/locale/brand.properties")
> +      .GetStringFromName("brandFullName");

The .properties file says that it expects brandShortname here.

Comment 12

[Benjamin Smedberg]

Attachment: bug1133000-flash-toggle.patch

Update: doing a "Learn More" link required a little bit of plumbing in the <setting> binding. Looks pretty straightforward to me, but I wanted somebody else to read it.

Comment 13

[Matthew N. [:MattN]]

Comment on attachment 8567293 [details] [diff] [review]
bug1133000-flash-toggle.patch

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

::: toolkit/mozapps/extensions/content/extensions.css
@@ +73,5 @@
>    display: -moz-grid-line;
>    -moz-binding: url("chrome://mozapps/content/extensions/setting.xml#setting-localized-bool");
>  }
>  
> +setting[type="bool"] .preferences-learnmore {

You could also do the following so you don't need to hard-code "visibility: visible;":
setting[type="bool"]:not([learnmore] .preferences-learnmore {
  visibility: collpase;
}

::: toolkit/mozapps/extensions/content/setting.xml
@@ +144,5 @@
>            <xul:label class="preferences-title" flex="1" xbl:inherits="xbl:text=title"/>
>          </xul:hbox>
>          <xul:description class="preferences-description" flex="1" xbl:inherits="xbl:text=desc"/>
> +        <xul:label class="preferences-learnmore text-link"
> +                   onclick="document.getBindingParent(this).openLearnMore()">&setting.learnmore;</xul:label>

You should probably check with UX about the desired location of the Learn More link. It seems to be on a new line below the description but I wonder if it should be immediately after the description or after the checkbox.

@@ +176,5 @@
> +
> +      <method name="openLearnMore">
> +        <body>
> +        <![CDATA[
> +          window.open(this.getAttribute("learnmore"), "_blank");

Why are you specifying "_blank"? I think it should open in a new tab by default without it.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/11b57818df63
https://hg.mozilla.org/mozilla-central/rev/973d369b4f28

Comment 15

[Théo Chevalier]

There is a typo in the string:

%S changed some Adobe Flash settings improve performance.
(Missing "to" before "improve")

Could you please land a quick fix before string freeze?

I'd update the entity name though, because as a localizer, I had to read c#8 before getting what the string really meant.

Comment 16

[Benjamin Smedberg]

Fixed typo at https://hg.mozilla.org/integration/mozilla-inbound/rev/cef15f16f5a5

I didn't change the name.

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cef15f16f5a5

Comment 18

[Matt Wobensmith [:mwobensmith][:matt:]]

Some questions:

* What benefit is this to someone whose Flash crashes are not caused by protected mode?

* If the user has been presented a choice - or the consequence of a choice made for them - to decrease security protection, is the gravity of this decision conveyed by the current messaging herein?

Comment 19

[Benjamin Smedberg]

Turning off protected mode decreases the general Flash crash and hang rate by more than half. We typically don't know whether a particular hang is caused by protected mode, but if it's not then this change may not benefit an individual user.

Users are not presented with a choice, just a notification, because we believe that users are unable to make an informed decision about this, and so we are making this decision as the user agent with the notification. The Learn More link will be the primary place we can provide additional context.

Comment 20

[Benjamin Smedberg]

Disabled at https://hg.mozilla.org/integration/mozilla-inbound/rev/4e1809fa5a22

Before doing something this drastic we're going to see if we can embed somebody full-time into an Adobe office to fix the most prevalent hang bugs.

Comment 21

[Benjamin Smedberg]

https://hg.mozilla.org/releases/mozilla-aurora/rev/488646d66485

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4e1809fa5a22