Closed
Bug 765506
Opened 12 years ago
Closed 12 years ago
click-to-play: attach click listener to plugin element instead of overlay
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox14 affected, firefox15 affected, firefox16 affected)
RESOLVED
FIXED
People
(Reporter: bugzilla, Assigned: keeler)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0 Build ID: 20120612164001 Steps to reproduce: I have the new click-to-play function enabled. When I would like to active the flash Plugin for a flash-object of the Site Sockshare.com and Putlocker.com, I can't do that. Actual results: When I try to activate the plugin by clicking on the button, nothing happens. The stent remains, and the video does not play itself. Also update a page does not work. Expected results: You should make the (wild card) away, and instead should activate the plugin and start running. For example on YouTube.
Updated•12 years ago
|
Component: Untriaged → Plug-ins
Product: Firefox → Core
QA Contact: untriaged → plugins
Assignee | ||
Comment 3•12 years ago
|
||
Looking at the page, this is another instance of bug 741130.
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
(In reply to David Keeler from comment #3) > Looking at the page, this is another instance of bug 741130. > > *** This bug has been marked as a duplicate of bug 741130 *** Are you sure? The bug you linked is about Firefox for Android. Is there something I missed?
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Loic from comment #4) > Are you sure? The bug you linked is about Firefox for Android. Is there > something I missed? Sorry, that was misleading - bug 741130 is indeed for Firefox for Android, but the exact same problem occurs in Firefox for desktop, so I was intending to fix both problems at the same time. A better approach might be to have separate bugs, since the fixes are technically separate.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Summary: Click-to-Play Plugin Bug by Sockshare and Putlocker (Flash) → click-to-play: attach click listener to plugin element instead of overlay
Assignee | ||
Updated•12 years ago
|
Blocks: click-to-play
Assignee | ||
Comment 6•12 years ago
|
||
Jared - this is the desktop version of bug 741130.
Comment 7•12 years ago
|
||
Comment on attachment 643436 [details] [diff] [review] patch Review of attachment 643436 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser-plugins.js @@ +285,2 @@ > gPluginHandler.activateSinglePlugin(aEvent.target.ownerDocument.defaultView.top, aPlugin); > + aPlugin.removeEventListener("click", this); aPlugin.removeEventListener("click", listener, true); ::: browser/base/content/test/browser_pluginnotification.js @@ +631,5 @@ > + var objLoadingContent = plugin.QueryInterface(Ci.nsIObjectLoadingContent); > + ok(!objLoadingContent.activated, "Test 19b, plugin should not be activated"); > + > + gTestBrowser.contentWindow.displayPlugin(); > + var condition = function() doc.getAnonymousElementByAttribute(plugin, "class", "mainBox") != null; If it's going from display:none to display:block, isn't the mainBox always null? ::: browser/base/content/test/plugin_hidden_to_visible.html @@ +7,5 @@ > +<script> > +function addPlugin() { > + var div = document.createElement("div"); > + div.id = "container"; > + div.style.display = "none"; I think the test should be having the object with display:none, not the container, right?
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #7) > If it's going from display:none to display:block, isn't the mainBox always > null? ... > I think the test should be having the object with display:none, not the > container, right? From the behavior I'm seeing, the mainBox is only null if the element the object is in is display:none. If the object itself is display:none, the overlay does exist - it's just not visible.
Comment 9•12 years ago
|
||
Comment on attachment 643436 [details] [diff] [review] patch Review of attachment 643436 [details] [diff] [review]: ----------------------------------------------------------------- Ok, I'm fine with the changes then, but make sure to update the removeEventListener line. ::: browser/base/content/browser-plugins.js @@ +276,5 @@ > return; > } > > + let listener = { > + handleEvent: function(aEvent) { Since there is only one event to handle, can you change this to a plain-old function then just reference the function in the add/removeEventListener calls?
Attachment #643436 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 10•12 years ago
|
||
Thanks for the speedy review - carrying over r+. Try run: https://tbpl.mozilla.org/?tree=Try&rev=5d5b136a5754
Attachment #643436 -
Attachment is obsolete: true
Attachment #643503 -
Flags: review+
Reporter | ||
Comment 11•12 years ago
|
||
15 Beta with Firefox, it still does not work. How can I use the patch that you have made; Sorry I'm not a developer and I understand little of it.
Comment 12•12 years ago
|
||
The patch passed all the tests on our tryserver, so it should be checked in to mozilla-central within a few days and then appear on Firefox Nightly the day after. As to the Version attribute of this bug, please leave it at Firefox 14 since that is the first version where this bug occurred. When this bug gets fixed it will have the "Target Milestone" updated to reflect which version that it initially got fixed in. If the fix is expedited to our Beta or Aurora branch, then the status-firefox14 and status-firefox15 flags will be updated.
status-firefox14:
--- → affected
status-firefox15:
--- → affected
status-firefox16:
--- → affected
Version: 15 Branch → 14 Branch
Updated•12 years ago
|
Keywords: checkin-needed
Comment 13•12 years ago
|
||
Sorry, removing checkin-needed as per https://bugzilla.mozilla.org/show_bug.cgi?id=741130#c22
Keywords: checkin-needed
Assignee | ||
Comment 14•12 years ago
|
||
This was fixed by bug 741130.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 12 years ago
Depends on: 741130
Resolution: --- → FIXED
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
•