326737 - Shift key ignored when opening link in new tab from context menu

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

The last parameter of http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/nsContextMenu.js#526 should be event.shiftKey instead of false.  Same for the openframeintab function below it.  This will require passing in an event to those functions, so http://lxr.mozilla.org/seamonkey/search?string=openlinkintab the command(s?) there will have to be fixed. Should still be pretty easy.

Might as well check to see who else calls openlinkintab without a useful third parameter.

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

Comment 2

[jag (Peter Annema)]

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

+        openNewTabWith( this.linkURL(), true, aEvent && "shiftKey" in aEvent && aEvent.shiftKey );
+        openNewTabWith( this.target.ownerDocument.location.href, true, aEvent && "shiftKey" in aEvent && aEvent.shiftKey );

Since you've fixed both places that call this to always pass in |event| you don't need the |aEvent && "shiftKey" in aEvent && | part.

Comment 3

[jag (Peter Annema)]

All three places (in two files).

Comment 4

[[e-mail]]

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

Or pass aEvent.shiftKey into openFooInTab.

Comment 5

[[e-mail]]

(which has the advantage of not breaking extensions - it will just default).

Comment 6

[jag (Peter Annema)]

Just in case we wanna handle other modifiers later on, if we pass in |event| now that won't be a problem then, but if we pass in a bool now then changing it to event later on will be harder. Just null check event and you should be good.

Comment 7

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch v2

Comment 8

[jag (Peter Annema)]

Comment on attachment 223185 [details] [diff] [review]
patch v2

s/aShiftKey/reverseBackgroundPref/

Follow |openNewTabWith()|s lead in the name of the argument, and since the rest of this file doesn't use the a prefix, let's not here either.

With that fixed r=jag

Comment 9

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Checked in on trunk with jag's nit.

Comment 10

[Robert Kaiser]

Comment on attachment 223185 [details] [diff] [review]
patch v2

a=me for SeaMonkey 1.1