168916 - Removing unecessary redundancy in nsContextMenu.js

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[TGOS]

I know that this is trivial, but wouldn't it be better coding to remove
reduntant checks from the nsContextMenu.js file? I mean why performing the same
operation multiple times instead of performing it just once and saving the
result to a temp variable?

Why using

this.showItem( "context-back", !( this.isTextSelected || this.onLink ||
this.onImage || this.onTextInput ) );
this.showItem( "context-forward", !( this.isTextSelected || this.onLink ||
this.onImage || this.onTextInput ) );

instead of

var showNav = !( this.isTextSelected || this.onLink || this.onImage ||
this.onTextInput );   
this.showItem( "context-back", showNav );
this.showItem( "context-forward", showNav );

???

Comment 1

[TGOS]

Attachment: Modified nsContextMenu.js

The attached nsContextMenu performs 2 and-operations, 7 not-operations and 35
or-operations less than the currently used one and that every time the context
menu is opened. But there is no difference in functionality.

Comment 2

[Christopher Hoess (gone)]

Could you please attach this as a patch against the current nsContextMenu.js? If
you don't use CVS, but are just working with chrome, I believe you can generate
it with Patch Maker. <URL:http://www.gerv.net/software/patch-maker/> Thanks.

Comment 3

[TGOS]

> Could you please attach this as a patch against the current nsContextMenu.js? If
> you don't use CVS, but are just working with chrome, I believe you can generate
> it with Patch Maker. <URL:http://www.gerv.net/software/patch-maker/> Thanks.

Can't run Patch Maker, have no Perl interpreter installed and CVS is kinda an
overkill for me to run (I have only a CVS tool in my Java IDE, which I never
used so far). However, what you want me to generate is a file one can use
together with the Linux/UNIX tool "patch", isn't it? Can't I generate such a
file using the "diff" command? As I have this command available on Windows
(thansk to Cygwin) and I could attach the output here as file if you like.

Comment 4

[Christopher Hoess (gone)]

A "diff -u" would be great, yes.

Comment 5

[TGOS]

Attachment: diff -u output

Here's the "diff -u" output that shows the changes which have been made.

Comment 6

[Steve Chapel]

Attachment: new fix

The old patch bitrotted -- here's an updated patch.

Comment 7

[Christopher Hoess (gone)]

Comment on attachment 119829 [details] [diff] [review]
new fix

r=choess

Comment 8

[jag (Peter Annema)]

Comment on attachment 119829 [details] [diff] [review]
new fix

sr=jag

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 119829 [details] [diff] [review]
new fix

a=sspitzer

Comment 10

[(not reading, please use [e-mail] instead)]

what happened here?  did this have r/sr/a for 1.4 beta, but never checked in?

Comment 11

[Christopher Hoess (gone)]

probably (I don't have checkin privs, and neither does Steve AFAIK)

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 119829 [details] [diff] [review]
new fix

a=asa (on behalf of drivers) for checkin to 1.4.

Comment 13

[[e-mail]]

Fix checked in.