883404 - Disable running plugins on the page when user re-blocks them

Status: VERIFIED FIXED

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

Description

[Larissa Co [:lco]]

This bug refers to section 3.4 (pg. 16) of my design spec: http://people.mozilla.com/~lco/CtP/130415%20CtP%20design%20spec.pdf

When the plugin is already enabled on the page, the user can go to the CtP icon to re-block the plugin. However, in order to do so, we need to reload the page. This action might cause loss in user data, which we need to warn the user about.

After trying to modify the text to explain to the user, I am back to the idea of changing the button text instead. Is it possible to say something like "Disable (Reloads Page)", or is that too long?

Other ideas:
Disable and Reload
Reload to Block

It doesn't say your data will be lost though...

Comment 1

[Benjamin Smedberg]

Other options:

B. Don't do anything, just mark the plugin blocked and let the user reload if necessary. Disadvantages: the user might not know why the plugin is still alive on the current page

C. Keep the doorhanger open after "Block" has been clicked and change the text, so e.g. 'Plugin "Oracle Java" has been blocked. Reload the page to complete blocking.' Auto-dismiss this after a timeout?

For Firefox 24 my plan is to nothing.

Comment 2

[John Schoenick [:johns]]

Is there a reason we don't want to just force-reload all matching plugins in the page, causing them to switch to fallback?

Comment 3

[Larissa Co [:lco]]

(In reply to John Schoenick [:johns] from comment #2)
> Is there a reason we don't want to just force-reload all matching plugins in
> the page, causing them to switch to fallback?

Our concern with automatically reloading the page was that anything that's unsaved on the page will disappear. But if there's a way to reload just the plugins... (I don't even know if that's feasible) that would be a good idea.

Comment 4

[John Schoenick [:johns]]

Attachment: Add a object.reload() call for chrome to de-activate plugins

(In reply to Larissa Co [:lco] from comment #3)
> Our concern with automatically reloading the page was that anything that's
> unsaved on the page will disappear. But if there's a way to reload just the
> plugins... (I don't even know if that's feasible) that would be a good idea.

I think that would work - this patch adds a simple tag.reload() call for chrome to clear that activation state and reload an <object> tag, then the front-end could do something like:

> for (let plugin of pluginsOnThePage) {
>   if (plugin.displayedType == TYPE_PLUGIN && plugin.actualType == MIMEInQuestion) {
>     plugin.reload(true);
>   }
> }

Comment 5

[Georg Fritzsche [:gfritzsche]]

(In reply to John Schoenick [:johns] from comment #4)
> (In reply to Larissa Co [:lco] from comment #3)
> > But if there's a way to reload just the
> > plugins... (I don't even know if that's feasible) that would be a good idea.
> 
> I think that would work - this patch adds a simple tag.reload() call for
> chrome to clear that activation state and reload an <object> tag, then the
> front-end could do something like:

I think the interesting question would be how many pages still remain (mysteriously for the user) broken afterwards due to the plugin and content-scripts being in different states, no?

Comment 6

[Benjamin Smedberg]

In this case I don't think we care. If the user chose to disable plugins, they will probably already be aware that the page might break and they might need to reload. I didn't realize that olc.reload() was a safe thing to do, but if it is let's try it.

johns wanna get review from josh?

Comment 7

[John Schoenick [:johns]]

Attachment: Add a object.reload() call for chrome to de-activate plugins

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> I didn't realize that olc.reload() was a safe thing to do,
> but if it is let's try it.

LoadObject() can be triggered by content and can re-enter into content, so it is safe to call at arbitrary points. (Here's a version that actually compiles)

Comment 8

[Josh Aas]

I see how this avoids data loss for HTML, but what if the user's content is in Flash, unsaved? A Flash text editor, for example, and they don't realize it and disable plugins. Won't they lose their work?

Comment 9

[Benjamin Smedberg]

Yes, but that's a natural consequence of the decision they made, and not something I think we should protect them from.

Comment 10

[Josh Aas]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> Yes, but that's a natural consequence of the decision they made, and not
> something I think we should protect them from.

Most users are unqualified to make that decision because they don't know all of the potential consequences, but I'm happy to defer to you on this and move forward.

Comment 11

[Larissa Co [:lco]]

(In reply to Josh Aas (Mozilla Corporation) from comment #10)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> > Yes, but that's a natural consequence of the decision they made, and not
> > something I think we should protect them from.
> 
> Most users are unqualified to make that decision because they don't know all
> of the potential consequences, but I'm happy to defer to you on this and
> move forward.

I think this is fine. The user path to blocking the plugin is likely going to be:

plugin auto-blocked on page > user enables plugin > user interacts with plugin > user decides this was a bad idea and re-blocks the plugin.

In most cases like this, the user is interacting with the plugin and can see the results of his actions.

Comment 12

[Timvde]

To solve the issue of losing data, we might give a visual clue when the user hovers "Block this plug-in". I propose to let them grey out with a text overlay, saying "This plug-in will be disabled".

Comment 13

[taxilian]

FWIW I agree with Benjamin on this one; if a user re-blocks a plugin it's akin to killing a task in windows.  Sure, show them a "are you sure? this could lose data!" confirm or something if you think it's needed, but if the user actually blocks the plugin I see no problem with the plugin being terminated immediately.

Comment 14

[John Schoenick [:johns]]

Comment on attachment 765002 [details] [diff] [review]
Add a object.reload() call for chrome to de-activate plugins

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9cbcd8206a0

Landed so this doesn't bitrot, but we still need a front-end patch

Comment 15

[Carsten Book [:Tomcat]]

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

Comment 16

[Benjamin Smedberg]

Attachment: (UI portion) - Disable running plugins on the current page when the user chooses "Block"

Sorry jschoenick, I didn't realize you were waiting on a UI patch.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 18

[Paul Silaghi, QA [:pauly]]

One problem:
Now after the plugin is re-blocked, clicking on the plugin overlay will no longer trigger the doorhanger popup. FF 28.0a1 (2013-11-14) win 7

Comment 19

[Georg Fritzsche [:gfritzsche]]

(In reply to Paul Silaghi, QA [:pauly] from comment #18)
> One problem:
> Now after the plugin is re-blocked, clicking on the plugin overlay will no
> longer trigger the doorhanger popup. FF 28.0a1 (2013-11-14) win 7

Can you file a blocking follow-up for that please?

Comment 20

[Andrei Vaida [:avaida]]

User agents:
[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
[2] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
[3] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:30.0) Gecko/20100101 Firefox/30.0

I was able to confirm the fix for this issue on:
- the latest Beta (Build ID: 20140210161136) [1]
- the latest Aurora (Build ID: 20140210004002) [2]
- the latest Nightly (Build ID: 20140211030201) [3]

Note: dependency Bug 939090 is still reproducible on all of the above.

Comment 21

[Andrei Vaida [:avaida]]

I was able to confirm the fix for this issue on the latest Beta as well (Build ID: 20140224220227) [1].

[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0