Closed Bug 883404 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox28 --- verified

People

(Reporter: lco, Unassigned)

References

Details

(Whiteboard: [CtPDefault:P3])

Attachments

(2 files, 1 obsolete file)

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...
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.
Is there a reason we don't want to just force-reload all matching plugins in the page, causing them to switch to fallback?
(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.
(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);
>   }
> }
(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?
Priority: -- → P3
Whiteboard: [CtPDefault:P2]
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?
Whiteboard: [CtPDefault:P2] → [CtPDefault:P3]
(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)
Attachment #765002 - Flags: review?(joshmoz)
Attachment #764395 - Attachment is obsolete: true
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?
Yes, but that's a natural consequence of the decision they made, and not something I think we should protect them from.
(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.
Attachment #765002 - Flags: review?(joshmoz) → review+
(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.
Blocks: 880735
Summary: Tell the user that the page will reload when they re-block a plugin → Disable running plugins on the page when user re-blocks them
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".
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 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
Attachment #765002 - Flags: checkin+
Whiteboard: [CtPDefault:P3] → [CtPDefault:P3][leave open]
Sorry jschoenick, I didn't realize you were waiting on a UI patch.
Attachment #829755 - Flags: review?(jaws)
Attachment #829755 - Flags: review?(jaws) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [CtPDefault:P3][leave open] → [CtPDefault:P3]
Target Milestone: --- → mozilla28
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
Flags: needinfo?
(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?
Flags: needinfo?
Depends on: 939090
Keywords: verifyme
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.
Keywords: verifyme
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
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.