Closed
Bug 812562
Opened 12 years ago
Closed 12 years ago
click-to-play blocklisted plugins: reshow urlbar notification as with normal click-to-play
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla20
People
(Reporter: keeler, Assigned: keeler)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
When navigating back, click-to-play events aren't re-fired*, so we manually check and see if we need to re-show the popup notification (albeit in the dismissed state, so we just see the icon in the urlbar). However, this code assumes if "plugins.click_to_play" is false, it doesn't need to do anything. With the advent of click-to-play blocklisting, this is no longer the case.
*This might change with bug 800018, but we'll cross that bridge when we come to it.
Assignee | ||
Updated•12 years ago
|
Blocks: click-to-play
Assignee | ||
Comment 1•12 years ago
|
||
Jared, if you could have a look at this, that'd be great.
As a side-note, I refactored browser/.../test/browser_pluginnotification.js a bit with the idea of stopping that file from growing so large.
Attachment #682565 -
Flags: review?(jaws)
Comment 2•12 years ago
|
||
Comment on attachment 682565 [details] [diff] [review]
patch
Review of attachment 682565 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with the explicit plugins.click_to_play=false set in the test.
::: browser/base/content/test/browser_bug812562.js
@@ +42,5 @@
> +function prepareTest(nextTest, url) {
> + gNextTest = nextTest;
> + gTestBrowser.contentWindow.location = url;
> +}
> +// Tests that the going back will reshow the notification for click-to-play
nit: trailing whitespace.
@@ +44,5 @@
> + gTestBrowser.contentWindow.location = url;
> +}
> +// Tests that the going back will reshow the notification for click-to-play
> +// blocklisted plugins (part 1/4)
> +function test1a() {
This test should explicitly set plugins.click_to_play=false since that is what it is expecting.
Attachment #682565 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Fixed issues. Carrying over r+.
https://tbpl.mozilla.org/?tree=Try&rev=8493a4c41628
Attachment #682565 -
Attachment is obsolete: true
Attachment #682664 -
Flags: review+
Assignee | ||
Comment 4•12 years ago
|
||
That had some issues, so I actually ended up getting rid of that fake blocklist business and replacing it with using the actual blocklist. I'd appreciate another look - thanks!
Try run: https://tbpl.mozilla.org/?tree=Try&rev=90fb28428f94
Attachment #682664 -
Attachment is obsolete: true
Attachment #683816 -
Flags: review?(jaws)
Updated•12 years ago
|
Attachment #683816 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment 7•12 years ago
|
||
Can you please tell me how to test this fix?
Assignee | ||
Comment 8•12 years ago
|
||
This should work on windows:
1. install an old version of flash (<10.2.9999)
2. replace "addons.mozilla.org" with "addons-dev.allizom.org" in the pref extensions.blocklist.url
3. evaluate `Components.classes["@mozilla.org/extensions/blocklist;1"].getService(Components.interfaces.nsITimerCallback).notify(null);' in the error console
4. visit a page with flash on it (verify that it's ctp-blocked and there is a urlbar notification)
5. navigate to a page without flash (in the same tab)
6. go back (verify that the urlbar notification shows up again)
Comment 9•12 years ago
|
||
Thanks David for explaining. But:
(In reply to David Keeler from comment #8)
> This should work on windows:
> 1. install an old version of flash (<10.2.9999)
why applies only to versions <10.2 ?
> 2. replace "addons.mozilla.org" with "addons-dev.allizom.org" in the pref
> extensions.blocklist.url
shouldn't be the same on production?
> 4. visit a page with flash on it (verify that it's ctp-blocked and there is
> a urlbar notification)
what notification exactly? Are you talking about that old info-bar saying the plugin is outdated? As far as I know it was removed after FF 16. I'm only seeing the CTP specific doorhanger near the location bar.
> 5. navigate to a page without flash (in the same tab)
> 6. go back (verify that the urlbar notification shows up again)
I don't see any differences before and after the fix.
Assignee | ||
Comment 10•12 years ago
|
||
Sorry - I was just trying to be specific about how to get a plugin to be click-to-play blocklisted. Let me try again:
1. Get a plugin to be click-to-play blocklisted
2. Visit a page with that plugin (verify the CTP-specific doorhanger is in the urlbar)
3. Navigate to a page without that plugin
4. Go back (verify that the CTP-specific doorhanger shows up again - before this was fixed, if the plugin was not regular click-to-play, it wouldn't show up)
Comment 11•12 years ago
|
||
Works fine on latest nightly, but I'm also not able to reproduce the issue before the fix (tested on Nightly 2012-11-17, Nightly 2012-11-26). A test url would help, if not much of a trouble.
Assignee | ||
Comment 12•12 years ago
|
||
Any youtube video should do the trick (if flash is ctp-blocked). Then for the page without a plugin, I've just been using www.google.com.
Comment 13•12 years ago
|
||
I'm still not able to reproduce the issue on the affected builds. Anyway, I don't think this makes any difference since everything looks ok after the fix.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•