207861 - Rename Folder option is useless in the newsgroup

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Pascal Chevrel:pascalc]

build 2003052508 WinXP

1 in MailNews right click on a newsgroup name
2 select "rename folder"

this option is useless for newsgroup which are not folder and has no effect,
Netscape 7.02 did not have this option

I think it should be removed for 1.4 on which Netcaspe will be built, this is
not a high risk change but useless UI shouldn't be shown into a commercial build

Comment 1

[Pascal Chevrel:pascalc]

This useless option wasn't in Ns7.02, adding "regression" keyword.

Comment 2

[Martin Strobl]

I would like the "rename folder" function to be functioning. I have subscribed
some newsgroups on the support-forums.novell.com server and as the newsgroups
are organized hierarchically there, the name of the group (including the path)
is too long. As the result, I am getting abbreviated forms of the groups and I
am not able to differentiate between them.

Example: 
for newsgroup named "novell.support.os.server.netware6x.install-upgrade"
I am getting "n.s.o.s.n.install-upgrade", which is unfortunatelly identical to
the   abbreviation for the "novell.support.os.server.netware5x.install-upgrade"
newsgroup.

Comment 3

[Pascal Chevrel:pascalc]

Since this is not a blocker for 1.4, I add a nsbeta1 keyword. Please, do not
release a Netscape 7.1 with a new context menu option which has no effect,
disable this option until there is some working code behind it!

Comment 4

[Samir Gehani]

adt: nsbeta1-

Comment 5

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Stop "Rename Folder" appearing in newsgroups.

Gerv

Comment 6

[Gervase Markham [:gerv]]

Taking bug.

Gerv

Comment 7

[Gervase Markham [:gerv]]

Comment on attachment 130970 [details] [diff] [review]
Patch v.1

Asking Seth for review

Comment 8

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

Comment on attachment 130970 [details] [diff] [review]
Patch v.1

why is the folder attribute "CanRename" true for a newsgroup?

that's unexpected, give this code:

http://lxr.mozilla.org/mozilla/source/mailnews/news/src/nsNewsFolder.cpp#473

we should find out what's going on here.

Comment 9

[Oleg Sidletskiy]

*** Bug 219998 has been marked as a duplicate of this bug. ***

Comment 10

[Robert Gelb]

I've seen some comments in here suggesting that the 'Rename Folder' context 
menu item be removed all together from the newsgroups.  I strongly disagree.  
Martin Strobl described the problem in comment #2.  I have a similar problem, 
except to a larger degree (many NGs appear identical).

It would be great, if one could assign an alias to a newsgroup - a killer 
feature for me.

Comment 11

[Pascal Chevrel:pascalc]

Then file a RFE bug to get newsgroup aliases, this is a different issue

Comment 12

[Markus Moebs]

Attachment: Patch almost identical to Patch v1

Comment 13

[Markus Moebs]

When will the patch finally be checked in? If you take a look at the code you
will see that somebody just forgot to add the && isMail condition in line 334 of
mailContextMenus.js because the variable isMail is set in the beginning of the
function but never used again. Ok, it is strange that canRename is true for
ngs... but why make a fool of Mozilla by keeping obviously nonsensical menuitems
that can be removed by changing one line in one file?

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

seth: the folder attribute isn't true. canRename becomes true because of this line:
   canRename = canRename || !isSpecialFolder;
isSpecialFolder is, of course, false.

this line was added for bug 190354.

I'm attaching a new patch that I like more than the current patch here.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

Comment 16

[chris hofmann]

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

a=chofmann for 1.7b

Comment 17

[Scott MacGregor]

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

asking Neil for the r=

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

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

neil points out that I can just remove this line

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: (Av3) patch v3 [Checked in: Comment 23]

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 143992 [details] [diff] [review]
(Av3) patch v3
[Checked in: Comment 23]

note, if you want this checked in today, I can't do it

Comment 21

[Scott MacGregor]

Comment on attachment 143992 [details] [diff] [review]
(Av3) patch v3
[Checked in: Comment 23]

carrying over the 1.7b approval for this patch

Comment 22

[Stephen Walker]

ShowMenuItem(...) needs to check isMail.

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 143992 [details] [diff] [review]
(Av3) patch v3
[Checked in: Comment 23]


Check in: { 03/17/2004 03:22	neil%parkwaycc.co.uk	mozilla/ mailnews/
base/ resources/ content/ mailContextMenus.js	     1.47 }

Comment 24

[Serge Gautherie (:sgautherie)]

(In reply to comment #22)
> ShowMenuItem(...) needs to check isMail.

Is an additional patch like
{{
- 333   ShowMenuItem("folderPaneContext-rename", (numSelected <= 1) && !isServer
&& !isSpecialFolder && canRename);

+ 333   ShowMenuItem("folderPaneContext-rename", (numSelected <= 1) && !isServer
&& (isMail && !isSpecialFolder) && canRename);
}}
needed, which would be more like |function SetupRemoveMenuItem(...)| ?

Otherwise, we should certainly remove the unused
{{
- 328   var isMail = serverType != 'nntp';
}}
(per comment 13)

Comment 25

[Stephen Walker]

Ignore comment 22. (Hint: test with clean sources next time)

Comment 26

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, this bug is fixed with neil's checkin. thanks for doing it for me.

Comment 27

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-SM) <mailContextMenus.js> (SeaMonkey part) [Checked in: Comment 30]

Unused |var isMail| removal, per previous comments.

Comment 28

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-TB) <mailContextMenus.js> (ThunderBird part) [Checked in: Comment 31]

Comment 29

[Serge Gautherie (:sgautherie)]

Comment on attachment 151842 [details] [diff] [review]
(Bv1-TB) <mailContextMenus.js> (ThunderBird part)
[Checked in: Comment 31]


Could you (super-)review/check in this patch ? Thanks.

Comment 30

[Serge Gautherie (:sgautherie)]

Comment on attachment 144270 [details] [diff] [review]
(Bv1-SM) <mailContextMenus.js> (SeaMonkey part)
[Checked in: Comment 30]


Check in: { 2004-06-28 01:43	neil%parkwaycc.co.uk	mozilla/ mailnews/
base/ resources/ content/ mailContextMenus.js	     1.51 }

Comment 31

[Scott MacGregor]

attachment 151842 [details] [diff] [review] checked into the 1.0 branch and the trunk for Thunderbird.
Thanks for the patch!

Comment 32

[Mike Cowperthwaite]

Verified with
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040710