Last Comment Bug 806136 - Fix code logic bug in addLinkClickCallback method (notification.xml)
: Fix code logic bug in addLinkClickCallback method (notification.xml)
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
-- normal (vote)
: seamonkey2.16
Assigned To: Frank Wein [:mcsmurf]
Depends on:
  Show dependency treegraph
Reported: 2012-10-27 14:35 PDT by Frank Wein [:mcsmurf]
Modified: 2012-11-11 05:21 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (805 bytes, patch)
2012-10-27 15:04 PDT, Frank Wein [:mcsmurf]
iann_bugzilla: review+
iann_bugzilla: approval‑comm‑aurora+
iann_bugzilla: approval‑comm‑beta+
Details | Diff | Splinter Review

Description User image Frank Wein [:mcsmurf] 2012-10-27 14:35:31 PDT
The addClickToPlayCallback method in notification.xml returns on a keydown event when the enter/return key is pressed:
                          function(aEvent) {
  if (aEvent.keyCode == aEvent.DOM_VK_RETURN)

This is wrong, the link should only be activated/followed when the enter key is pressed, not the other way round.
Comment 1 User image Frank Wein [:mcsmurf] 2012-10-27 15:04:22 PDT
Created attachment 675899 [details] [diff] [review]

Easy to review, also see about the code logic (this is where this code copied from, but the logic was not fixed when changing this part of the code).
Comment 2 User image Frank Wein [:mcsmurf] 2012-10-27 15:12:29 PDT
To test this patch I recommend the following way: Disable any plugin via the Add-On manager (for example Flash or Adobe PDF) and then go to a page with an embedded Flash animation or a pdf doc (for example Then select the "Manage plugins..." link (either with the tab key or hold the left mouse key over the link and then move away so that it doesn't click). As soon as you press any key then, it opens the Add-On manager (even enter works as this triggers the onClick handler in the same file :).
Comment 3 User image Ian Neal 2012-11-03 09:20:46 PDT
Comment on attachment 675899 [details] [diff] [review]

>-                                        if (aEvent.keyCode == aEvent.DOM_VK_RETURN)
>+                                        if (!(aEvent.keyCode == aEvent.DOM_VK_RETURN))
Wouldn't using != be better?
r=me with that fixed.
Comment 4 User image Frank Wein [:mcsmurf] 2012-11-08 17:22:23 PST
Pushed with change from Comment 3:

Though I wonder if that key handler there is actually required as the enter key also seems to fire a onclick event?
Comment 5 User image Frank Wein [:mcsmurf] 2012-11-10 04:11:59 PST
Comment on attachment 675899 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: Unexpected behavior in the plugin click-to-play UI when a link in the UI is selected and the user presses any key on his keyboard (for example to switch to another tab, pressing the Windows key or whatever). A link in the click-to-play GUI appears when the user the CTP enabled ("click here to activate plugin"), when a plugin has been marked as vulnerable (then the plugin gets switched to CTP) or when a plugin has crashed ("Reload Page" link).
Testing completed (on m-c, etc.): Patch has been tested on comm-central
Risk to taking this patch (and alternatives if risky): Almost no risk.
String changes made by this patch: None
Comment 6 User image Frank Wein [:mcsmurf] 2012-11-11 05:18:57 PST
Hrm, I used the wrong check-in comment on -central and -aurora :) (wrong function name, see summary change)

Pushed to comm-aurora:
Pushed to comm-beta:

Note You need to log in before you can comment on or make changes to this bug.