Closed Bug 806136 Opened 7 years ago Closed 7 years ago

Fix code logic bug in addLinkClickCallback method (notification.xml)


(SeaMonkey :: General, defect)

Not set


(seamonkey2.14 fixed, seamonkey2.15 fixed, seamonkey2.16 fixed)

Tracking Status
seamonkey2.14 --- fixed
seamonkey2.15 --- fixed
seamonkey2.16 --- fixed


(Reporter: mcsmurf, Assigned: mcsmurf)



(1 file)

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.
Attached patch PatchSplinter 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).
Assignee: nobody → bugzilla
Attachment #675899 - Flags: review?(iann_bugzilla)
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 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.
Attachment #675899 - Flags: review?(iann_bugzilla) → review+
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?
Target Milestone: --- → seamonkey2.16
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
Attachment #675899 - Flags: approval-comm-beta?
Attachment #675899 - Flags: approval-comm-aurora?
Attachment #675899 - Flags: approval-comm-beta?
Attachment #675899 - Flags: approval-comm-beta+
Attachment #675899 - Flags: approval-comm-aurora?
Attachment #675899 - Flags: approval-comm-aurora+
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:
Closed: 7 years ago
Resolution: --- → FIXED
Summary: Fix code logic bug in addClickToPlayCallback method (notification.xml) → Fix code logic bug in addLinkClickCallback method (notification.xml)
OS: Windows XP → All
Hardware: x86 → All
You need to log in before you can comment on or make changes to this bug.