Closed
Bug 831365
Opened 12 years ago
Closed 12 years ago
Unable to whitelist a site to disable plugin click-to-play, if Navigation Toolbar (or Location Bar) has been removed (e.g. in a locked-down kiosk type of situation)
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla21
People
(Reporter: dholbert, Assigned: jaws)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jaws
:
review+
keeler
:
feedback+
|
Details | Diff | Splinter Review |
STR:
1. Ensure you have Flash installed. (just as a demo plugin)
2. In about:config, set "plugins.click_to_play" to true.
(This is to simulate us pushing out a CTP block of a particular plugin.)
3. Visit http://homestarrunner.com/
--> Notice that location bar has a Plugin icon, w/ possibility to
always activate plugins for this site.
4. Go to View | Toolbars, and uncheck "Navigation Toolbar". Ctrl+R to reload. (optional)
EXPECTED RESULTS: There should still be some way to whitelist the site.
ACTUAL RESULTS: There's no way to whitelist the site.
Reporter | ||
Comment 1•12 years ago
|
||
For reference, see this dev.apps.firefox post:
https://groups.google.com/d/msg/mozilla.dev.apps.firefox/14cN-K0n8Bk/-ZCJqfJWcM4J
Reporter | ||
Updated•12 years ago
|
Summary: Unable to whitelist a site to disable plugin click-to-play, if Navigation Toolbar (or Location Bar) has been removed → Unable to whitelist a site to disable plugin click-to-play, if Navigation Toolbar (or Location Bar) has been removed (e.g. in a locked-down kiosk type of situation)
Comment 2•12 years ago
|
||
How is this different from other things that we hang off the location bar, like the security UI? In normal operation, the answer to this is probably "turn on the navigation bar, set your prefs, then turn off the navigation bar".
Reporter | ||
Comment 3•12 years ago
|
||
I think this is different because, for e.g. a kiosk that's at a start page & only shows a limited set of pages, you don't care about security UI or other things that hang off of the location bar. You do, however, care about your plugins working reliably.
> In normal operation, the answer to this is probably "turn on the navigation bar, set
> your prefs, then turn off the navigation bar".
That will work around it, yeah. I'm not convinced that this (browser w/out navbar) is a use-case we need to extensively support; I just filed this bug because you recommended doing so, in the thread in comment 1. :)
Reporter | ||
Comment 4•12 years ago
|
||
Note: If instead of homestarrunner, you visit a site like http://soundcloud.com with _hidden_ plugins, *then* a doorhanger automatically pops out of the tab and lets you whitelist the site (even though there's no navbar).
I'm guessing we hide that doorhanger (behind the plugin icon in the location bar) if we know the plugin is visible. Perhaps, if the location bar is hidden, we should show the doorhanger unconditionally?
Updated•12 years ago
|
Blocks: click-to-play
Assignee | ||
Comment 5•12 years ago
|
||
This is a simple patch that will just always show the doorhanger in this scenario.
Comment 6•12 years ago
|
||
Comment on attachment 703769 [details] [diff] [review]
Patch
This seems reasonable - the PopupNotifications API already handles the anchor not being visible by falling back to anchoring notifications on the tab.
Attachment #703769 -
Flags: feedback+
Comment 7•12 years ago
|
||
Though I would probably use isElementVisible(gURLBar) instead, since IIRC the anchor is in the location bar (which may not be on the nav-bar).
Assignee | ||
Comment 8•12 years ago
|
||
Thanks for pointing out isElementVisible.
Attachment #703769 -
Attachment is obsolete: true
Attachment #703769 -
Flags: review?(felipc)
Attachment #705965 -
Flags: review?(gavin.sharp)
Comment 9•12 years ago
|
||
Comment on attachment 705965 [details] [diff] [review]
Patch v1.1
Looks like you're missing a "!"!
r=me with that fixed, and this manually tested. But I don't know if always-showing-dismissed has other implications, or whether this fix is sufficient, and I don't have time to chase that down, so get someone more familiar with the rest of this doorhanger logic (keeler?) to sign-off too.
Attachment #705965 -
Flags: review?(gavin.sharp)
Attachment #705965 -
Flags: review+
Attachment #705965 -
Flags: feedback?(dkeeler)
Assignee | ||
Comment 10•12 years ago
|
||
David, does this look alright to you?
Note that showing the doorhanger like this causes it to flicker when there are multiple plugins on the page. This is a separate bug that needs to be filed, but I'd rather not include it with this fix. The cause of that bug is because the doorhanger contents are updated each time a PluginClickToPlay event is received and the doorhanger is reshown (we don't need to do this when there are no new plugin types).
Attachment #705965 -
Attachment is obsolete: true
Attachment #705965 -
Flags: feedback?(dkeeler)
Attachment #705976 -
Flags: review+
Attachment #705976 -
Flags: feedback?(dkeeler)
Comment on attachment 705976 [details] [diff] [review]
Patch v1.2
Review of attachment 705976 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good.
Here's my understanding of where it will automatically show:
- when a new CTP plugin element is added to the page
- when navigating back to a page with CTP plguins (same thing, really)
- when one plugin is played but there are still others that are CTP (this would probably be really annoying, but there's not much we can do about it, since there still needs to be a way to activate them)
Attachment #705976 -
Flags: feedback?(dkeeler) → feedback+
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Comment 13•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 14•12 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #5)
> Created attachment 703769 [details] [diff] [review]
> Patch
>
> This is a simple patch that will just always show the doorhanger in this
> scenario.
This works as expected in Nightly 21.0a1 (2013-01-25).
But, IMO, this is very annoying for other sites that are not whitelisted.
Comment 15•12 years ago
|
||
If someone has set up a Kiosk and stripped away the UI affordances for control it should be the Kiosk maker's responsibility to make things work. Either they don't want plugins in their content (so they're disabled/uninstalled, CTP irrelevant) or they've fully enabled the plugins to avoid CTP or at the very least whitelisted the sites the Kiosk is designed to browse.
If it's just people wanting more screen real estate and not an actual dedicated kiosk then full-screen mode is better for them: even more space, and all the UI is just off the top waiting to come back if you mouse near it.
Reporter | ||
Comment 16•12 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #15)
> or they've fully enabled the plugins
> to avoid CTP or at the very least whitelisted the sites the Kiosk is
> designed to browse.
As noted by jaws on the m.d.a.firefox thread, whitelisting doesn't work if we've pushed out a CTP-block for your plugin-version.
https://groups.google.com/d/msg/mozilla.dev.apps.firefox/14cN-K0n8Bk/H9SJIGnzOQ0J
Reporter | ||
Comment 17•12 years ago
|
||
(sorry, I missed the context -- I initially thought comment 15 was an objection to fixing this bug, but now it looks like it was a response to Comment 14. I agree with it as a response to comment 14 -- IMHO it's fine for this situation to be "very annoying", since (as dveditz said) it's the operator's responsibility to make sure their plugins are up-to-date, they have the browser UI customized as-desired, and they have plugins allowed/whitelisted as-desired. At that point, if it's "annoying" to click & make vulnerable versions of plugins run, I think that's probably fine & good. :))
Comment 18•12 years ago
|
||
I saw the comment in the dev.apps.firefox thread that we ignore the "always allow" permission when blocklisted. I object to that, we're ignoring an expressed user preference. I'd much rather take the risk that the user's internal work site will get hacked than the risk that the user disables the blocklisting mechanism entirely to get around us.
Is the old severity=0 "you're using an outdated plugin" infobar code completely gone? If not maybe we could resurrect that warning for the case where the blocklist says CTP but the user has said "allow for this site".
Of course there's the problem that the user may have intended to allow Java but is then hit with a Flash exploit or vice versa, but we're addressing that issue in other bugs.
Comment 19•12 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #17)
> I initially thought comment 15 was an objection to fixing this bug,
> but now it looks like it was a response to Comment 14.
> [...]
> At that point, if it's "annoying" to click & make vulnerable versions
> of plugins run, I think that's probably fine & good. :))
Actually I was objecting to fixing this bug. "Kiosk mode" is an attempt to lock down and remove user control from their browser, to keep users from doing "bad stuff" as defined by the Kiosk operator. But if the Kiosk operator is taking control from users then it's their responsibility to configure the Kiosk to work correctly. I would imagine most kiosk creators would NOT want their users being able to launch vulnerable plugins.
The root of this bug is the decision to ignore the expressed user choice to allow plugins on a particular site. The fix in this bug tries to give some of that choice back so I guess it's a necessary band-aide. I hope it's temporary because it's going to be annoying and isn't the real problem.
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•