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)
13.44 KB,
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
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff0511be54a6
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ff0511be54a6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment 7•11 years ago
|
||
Can you please tell me how to test this fix?
Assignee | ||
Comment 8•11 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•11 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•11 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•11 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•11 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•11 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
•