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

bug 754472 comment 41:
(In reply to Paul Silaghi [QA] from comment #41)
> Nightly 2012-12-10 Win 7 x64
> 1. Enable CTP
> 2. Open http://wickedgoodgames.com/java2/asteroids.html
> 3. Click the CTP doorhanger
> 
> Actual results:
> Java is not on the list
> http://img24.imageshack.us/img24/181/ctpjava.png
> 
> Any ideas? Works fine with other pages like
> http://mozqa.com/data/firefox/plugins/plugin-test.html

After a bit of investigation, it looks like if a plugin of a different type than those already present is added to the page after the others have been handled, the popup notification won't get updated to include the new type.

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

This should do it. Unfortunately, it also means having a whole bunch of the same type of plugin objects on the same page will result in a lot of wasted work. I'm thinking a cached list of what types are already on the page could help? But maybe that's over-complicating things.

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

Comment on attachment 691073 [details] [diff] [review]
patch

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

::: browser/base/content/test/browser_bug820497.js
@@ +35,5 @@
> +    var notification = PopupNotifications.getNotification("click-to-play-plugins", gTestBrowser);
> +    ok(notification, "should have popup notification");
> +    is(notification.options.centerActions.length, 1, "should be 1 type of plugin in the popup notification");
> +  }
> +  else if (gNumPluginBindingsAttached == 2) {

} else if (...) {

@@ +49,5 @@
> +    gBrowser.removeCurrentTab();
> +    window.focus();
> +    finish();
> +  }
> +  else {

} else {

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

Had to rebase. Carrying over r+.
Try run: https://tbpl.mozilla.org/?tree=Try&rev=24edbe6acce5
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/3aa1bed744bb

	Ed Morley [:emorley]
	Comment 5 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/3aa1bed744bb

	Paul Silaghi, QA [:pauly]
	Comment 6 • 12 years ago

Verified fixed on Nightly 2012-12-20 Win 7 x64, Ubuntu 12.04 and Mac OS X 10.8.2

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

Marking tracking-firefox19? as per bug 809846 comment 32 and 33.

	Alex Keybl [:akeybl]
	Comment 8 • 12 years ago

Please prepare patches for Aurora/Beta/ESR17 so that we can land before the end of the week.

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): click-to-play
User impact if declined: users won't be able to use the door-hanger to activate plugins added to the page dynamically
Testing completed (on m-c, etc.): landed in 20
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

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

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: needed for click-to-play
User impact if declined: see previous comment
Fix Landed on Version: 20
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

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

This is fixed in 20, so no need for an aurora patch. I'm carrying over r+ because the changes are not functional - just to make them apply cleanly to the different branches.

	Paul Silaghi, QA [:pauly]
	Comment 12 • 12 years ago

verified FF 20 in comment 6

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 13 • 12 years ago

Comment on attachment 706048 [details] [diff] [review]
patch for beta

verified fixed, low risk, approving.

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

beta: https://hg.mozilla.org/releases/mozilla-beta/rev/5b312c4d8130
esr17: https://hg.mozilla.org/releases/mozilla-esr17/rev/c732d17352f5

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

Shoot - I didn't mean to change the status-firefox20 flag. Sorry about that, Paul.

	Paul Silaghi, QA [:pauly]
	Comment 16 • 12 years ago

Verified fixed FF 19b4 Win7, Ubuntu 12.04, Mac OS X 10.7.5

	u279076
	Comment 17 • 12 years ago

Adding verifyme keyword as this will need verification in the next 17esr release.

	u279076
	Comment 18 • 12 years ago

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #17)
> Adding verifyme keyword as this will need verification in the next 17esr
> release.

Paul, if you don't want to wait for the release you can verify this with the latest 17esr nightly.

	Paul Silaghi, QA [:pauly]
	Comment 19 • 12 years ago

Verified fixed 2013-01-30-03-45-01-mozilla-esr17 Win7, Ubuntu 12.04, Mac OS X 10.8.2