Daniel Holbert [:dholbert] Reporter
	Description • 12 years ago

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.

	Daniel Holbert [:dholbert] 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

	Benjamin Smedberg
	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".

	Daniel Holbert [:dholbert] 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. :)

	Daniel Holbert [:dholbert] 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?

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 5 • 12 years ago

This is a simple patch that will just always show the doorhanger in this scenario.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	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).

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 8 • 12 years ago

Thanks for pointing out isElementVisible.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	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.

	Jared Wein [:jaws] (please needinfo? me) 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).

	Dana Keeler (she/her) (use needinfo) (:keeler for reviews)
	Comment 11 • 12 years ago

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)

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 12 • 12 years ago

Thanks.

https://hg.mozilla.org/integration/mozilla-inbound/rev/798608217a81

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/798608217a81

	Paul Silaghi, QA [:pauly]
	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.

	Daniel Veditz [:dveditz]
	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.

	Daniel Holbert [:dholbert] 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

	Daniel Holbert [:dholbert] 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. :))

	Daniel Veditz [:dveditz]
	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.

	Daniel Veditz [:dveditz]
	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.