751809 - click does not work on a plugin content when a plugin is just activated

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Dennis Lichtenthäler]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/15.0 Firefox/15.0a1
Build ID: 20120503030512

Steps to reproduce:

* Enable plugins.click_to_play in about:config
* Switch to a page with embedded flash content (i.e. youtube)
* Click "Click here to activate plugins." 


Actual results:

The flash content plays but interaction is not possible. The elements react to hovering correctly but clicking does not have any effect. Keyboard controls (tabbing through available elements) works however.
Once I switch to another tab and back, clicking works.


Expected results:

After activating the plugin, clicking should work.

Comment 1

[Cam]

Still present in FirefoxNightly 16.0a1 2012-06-13.

Comment 2

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

I reproduce this bug still on http://hg.mozilla.org/mozilla-central/rev/bb4b37094b9f.

(In reply to Dennis Lichtenthäler from comment #0) 
> Once I switch to another tab and back, clicking works.

I focus the other element in a same page, clicking works too.

Comment 3

[yukikeiichi]

This bug happened:
* Firefox Beta    14.0b8
* Firefox Aurora  15.0a2 2012-06-23
* Firefox Nightly 16.0a1 2012-06-23

This bug should fix before release Firefox 14.

Comment 4

[Alex Keybl [:akeybl]]

(In reply to Yuki Keiichi from comment #3)
> This bug happened:
> * Firefox Beta    14.0b8
> * Firefox Aurora  15.0a2 2012-06-23
> * Firefox Nightly 16.0a1 2012-06-23
> 
> This bug should fix before release Firefox 14.

The setting is not yet user facing yet, so no need to track for release. Sending over to Jared for initial investigation.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

To note, I have only heard this reported with youtube.com.

I'm not sure what is happening there, but I talked with Margaret and we think that we should move the click-to-play event handler to the actual plugin instead of the overlay (bug 741130). That might fix this bug.

Re-assigning to dkeeler since he is going to work on fixing bug 741130.

Comment 7

[Dennis Lichtenthäler]

(In reply to Jared Wein [:jaws] from comment #5)
> To note, I have only heard this reported with youtube.com.
Yep, I'm unable to reproduce this with anything other than youtube videos.

Comment 8

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Dennis Lichtenthäler from comment #7)
> Yep, I'm unable to reproduce this with anything other than youtube videos.

I'm unable to reproduce this too. (on Today's nightly, http://hg.mozilla.org/mozilla-central/rev/e61399f31505 )

Comment 9

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to OHZEKI Tetsuharu [:saneyuki_s] from comment #8)
> (In reply to Dennis Lichtenthäler from comment #7)
> > Yep, I'm unable to reproduce this with anything other than youtube videos.
> 
> I'm unable to reproduce this too. (on Today's nightly,
> http://hg.mozilla.org/mozilla-central/rev/e61399f31505 )

Oops! Sorry, I have missed the test.
This bug is REPRODUCED on Today's nightly on OSX 10.7, http://hg.mozilla.org/mozilla-central/rev/e61399f31505.

Comment 11

[Devdatta Akhawe [:devd]]

I can reproduce this too (Aurora 16.0a2 (2012-07-22))

Comment 12

[Georg Fritzsche [:gfritzsche]]

This is bug is due to "mouseup" dom events not being sent to the plugin due to this check:
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginInstanceOwner.cpp#2098

Before the click is delivered, mContentFocused should be set to true here:
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginInstanceOwner.cpp#2083

However the "focus" dom event is never delivered to the pluginInstanceOwner with click-to-play. 
Still looking into how to properly fix this.

Comment 13

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix focus issue with click-to-play

With click-to-play enabled, the focus event was delivered before the actual plugin instance owner was loaded.
Fixed by syncing the focus state when the frame is set on the plugin instance owner.

Comment 14

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 649335 [details] [diff] [review]
Fix focus issue with click-to-play

Josh, can you review this?

Comment 15

[Benjamin Smedberg]

This could really use a mochitest.

Comment 16

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 649335 [details] [diff] [review]
Fix focus issue with click-to-play

Good point, post-poning review request until this has a test.

Comment 17

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix & test for focus issue with click-to-play

Comment 18

[Georg Fritzsche [:gfritzsche]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=de8ccadbf88d

Comment 19

[Georg Fritzsche [:gfritzsche]]

Attachment: Fix & test for focus issue with click-to-play, v3

The test failed on try, even on 10.6 which runs the test fine for me. 
Adjusted successive click tests to do the same thing to find out:
https://tbpl.mozilla.org/?tree=Try&rev=9786a523e1d9

Comment 20

[Georg Fritzsche [:gfritzsche]]

"to (In reply to Georg Fritzsche [:gfritzsche] [away 11-19 Aug] from comment #19)
> Adjusted successive click tests to do the same thing to find out:
... if this is a problem with the test enviroment.

Comment 21

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch v4

Figured out the issue with the test not working. Georg said he'd be out for a bit, so I'll be shepherding this to landing. Asking for review from Josh.

Comment 22

[Josh Aas]

Comment on attachment 651559 [details] [diff] [review]
patch v4

Excellent fix, thanks Georg and David!

Comment 23

[Dana Keeler (she/her) (use needinfo) [:keeler]]

This was the run: https://tbpl.mozilla.org/?tree=Try&rev=e88d375f6e57
Marking checkin-needed.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

(In reply to David Keeler from comment #23)
> This was the run: https://tbpl.mozilla.org/?tree=Try&rev=e88d375f6e57

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6943b37c68cc

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6943b37c68cc

Comment 26

[Ed Morley [:emorley]]

Sorry, backed out for bug 783723:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1a745a45f2

Comment 28

[Georg Fritzsche [:gfritzsche]]

Attachment: patch v5

* Fix intermittent failures through SimpleTest.waitForFocus()
* Avoid causing failures on subsequent tests if the test for this bug fails

Josh, can i carry over the r+ for this fixed patch?

Comment 29

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #28)
> Created attachment 655568 [details] [diff] [review]
> patch v5

Try run for this patch:
https://tbpl.mozilla.org/?tree=Try&rev=7d088f4af68d

Comment 30

[Josh Aas]

(In reply to Georg Fritzsche [:gfritzsche] from comment #28)

> Josh, can i carry over the r+ for this fixed patch?

Yes

Comment 31

[Ryan VanderMeulen [:RyanVM]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #29)
> Try run for this patch:
> https://tbpl.mozilla.org/?tree=Try&rev=7d088f4af68d

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/31a27d47d242

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/31a27d47d242

Comment 33

[Benjamin Smedberg]

This is important enough for CTP that it should be uplifted, I believe: see bug 792550 which is probably a dup and affects embedded youtube videos, which are one of the primary use cases that should work.

Comment 34

[Alex Keybl [:akeybl]]

Georg - please nominate for Aurora approval when you get the chance.

Comment 35

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 655568 [details] [diff] [review]
patch v5

[Approval Request Comment]
Bug caused by: Click-to-play feature when active
User impact if declined: E.g. Youtube Flash not responding to button click unless switching tabs or otherwise refocusing plugin element.
Testing completed (on m-c, etc.): Been on nightly for a while, no negative feedback so far. Also verified locally.
Risk to taking this patch (and alternatives if risky): Low-risk, worst-case is probably causing other focus issues.
String or UUID changes made by this patch: none.

Comment 36

[Georg Fritzsche [:gfritzsche]]

Attachment: Aurora patch

Comment 37

[Georg Fritzsche [:gfritzsche]]

Attachment 664068 [details] [diff] needs checkin on Aurora.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/20a6963e3018

Comment 39

[Virgil Dicu [:virgil] [QA]]

After following the steps to reproduce from comment 0, I could not interact with the flash content on www.youtube.com or flash trailers in imdb.com for the build ID: 20120503030512.

For the same steps in comment 0 for 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0 beta 4
Build ID: 20121031065642
flash content is working on every mouse action. Also other flash content sites like trailers on imdb.com or vimeo.com.

Comment 40

[Paul Silaghi, QA [:pauly]]

Video controls are working fine with CTP enabled on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:18.0) Gecko/18.0 Firefox/18.0. Verified fixed