779662 - click-to-play: click handler on overlay too restrictive

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

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

Basically, I did this wrong and now I'm fixing it. The patch should be pretty self-explanatory.

Comment 1

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

Attachment: patch

Comment 2

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

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

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

::: browser/base/content/browser-plugins.js
@@ +279,5 @@
>      // The overlay is null if the XBL binding is not attached (element is display:none).
>      if (overlay) {
>        overlay.addEventListener("click", function(aEvent) {
> +        // Have to check that the target is not the link to update the plugin
> +        if (!(aEvent.target instanceof HTMLAnchorElement) && 

Can you see if aEvent.originalTarget will work better here? See https://developer.mozilla.org/en-US/docs/DOM/event.originalTarget and https://developer.mozilla.org/en-US/docs/DOM/event/Comparison_of_Event_Targets#Use_of_explicitOriginalTarget_and_originalTarget for more information.

Also, can you include a test for this that checks clicking on the image, non-link text, and link text in the overlay?

Comment 3

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

Attachment: patch v2

I was having trouble with EventUtils.synthesizeMouse until I figured out how it actually works and how to use it. In light of this, I decided it would be best to a) always pass in gTestBrowser.contentWindow and b) use synthesizeMouseAtCenter in all cases except where we specifically don't want to click the center. Let me know if this is too much scope creep, though.

Comment 4

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

https://tbpl.mozilla.org/?tree=Try&rev=2f25fd81809e ran green - marking checkin-needed.

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/532ff9ae12b8

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/532ff9ae12b8