102284 - File | New | Folder menu needs to be disabled while offline

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Backend, defect)

Description

[ji]

Build: 09/28 branch builds
OS: RH7.1-J, XP-J, Mac OS X

After going offline, I'm unable to create an IMAP folder.
Steps to reproduce:
1. On an IMAP account, set several folders for offline use.
2. On Account manager, check on "When I create new folders, select them for
offline" on Offline & Disk Space pane.
3. Select File | Offline | Work Offline
4. Select File | New | Folder (Folder is not grayed out).
   Enter a folder name and select the IMAP account as the parent folder.
5. Click on OK button, nothing happens. No folder is created. Click on Cancel
button can close new Folder window.

Comment 1

[David :Bienvenu]

the new folder command should be disabled. I believe that bug was filed on mohan
or dianesun - I'll have to go look for the dup.

Comment 2

[ji]

If I can't create new folder when offline, the checkbox of "When I create new
folders, select them for offline" on Offline & Disk Space pane doesn't make
sense to me.

Comment 3

[David :Bienvenu]

the checkbox says that when you create a new folder, you want it automatically
configured for offline use. That has nothing to do with being able to create
folders *while* you're offline. 

Comment 4

[ji]

Understood. After I go back to online and create a new folder, it does select
the new folder for offline use. 

Comment 5

[ji]

Delete and Rename has been disabled (bug 81321), but not this one.

Comment 6

[scottputterman]

reassigning to ssu.

Comment 7

[grylchan]

It would need to be disabled in the following:
 -Main File menu in messenger
 -in the file menu in both a downloaded and non-downloaded stand
  alone message window
 -context menu for any folder
  -Right click, New folder

Comment 8

[grylchan]

As part of the mail qa's bug triage day, this is still present
in 2001-12-14-09-trunk build

Comment 9

[David :Bienvenu]

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

Comment 10

[Calvin Liu]

I posted a patch. who can review it?
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=75934

Comment 11

[Calvin Liu]

Attachment: simply disable the context menu when offline

Comment 12

[grylchan]

I assume this bug will disable all features of
creating new folder if you are offline.
If so it would need to be disabled in:
  -file menu of messenger
  -context menu of account ('New folder')
  -context menu of folder ('New folder')
  -context menu of subfolder ('New subfolder')
  -file menu of stand alone mesg window

Comment 13

[timeless]

if the patch were correct, i'd suggest:
  EnableMenuItem("folderPaneContext-new", !ioService.offline);
instead of the lines we have.

but, gchan is right someone needs to convert all of the oncommand="someJSFn()" 
into command="cmdNewFolder" and have a <command id="cmdNewFolder" 
oncommand="someJSFn()"/>

Sorry, inspector just crashed on me, so i can't give the correct names, but it 
should be easy enough for someone else to do the research.

Comment 14

[Calvin Liu]

Attachment: disable the context menu when offline (except local folders)

I can't find "new folder" or something like this in a stand along message
window.
Why I can't obsolete the old patch which I posted yesterday? How can I get this
permission?

Comment 15

[Calvin Liu]

Attachment: new patch created with "-u" switch

regularized

Comment 16

[Sean Su]

Comment on attachment 77013 [details] [diff] [review]
new patch created with "-u" switch

I applied the patch and noticed that the context menu for "New Folder" (inbox)
changed to "New subfolder", and it did not disable when in offline mode.

Also the "New subfolder" menu item in the context menu for a regular folder
(ie, not inbox or trash), did not disable when in offline mode.  Text stayed
the same.

The File|New|Folder did disable correctly.

Comment 17

[Calvin Liu]

I applied this patch. There is a incompatible in this patch that I forgot - no
"folderOutliner" now - replaced by "folderTree".

Another key point is you must delete this line in menuContextMenus.js:
-  EnableMenuItem("folderPaneContext-new", true);

Please try again. Thanks.

Comment 18

[Calvin Liu]

Attachment: change "folderOutliner" to "folderTree"

Comment 19

[Calvin Liu]

Attachment: missed a "folderOutliner", sorry

How can I get the obsolete permission?

Comment 20

[Matthew T (active 1999-2002)]

[Permission granted.]

Comment 21

[Sean Su]

Comment on attachment 77594 [details] [diff] [review]
missed a "folderOutliner", sorry

looks better.  offline for IMAP accounts looks like it's working correctly for
the following menu items:
  New Folder
  Compact This Folder
  New subfolder
  Rename folder
  Delete folder
  File|New|Folder

However, when in offline mode for POP3 accounts, the following is disabled when
I think should be left enabled since it's POP3:
  New Folder
  New Subfolder
  File|New|Folder

Also, I still had problems applying the patch.	I had to manually apply parts
of it.	It's not a problem for me, but just wanted to let you know.

Comment 22

[Calvin Liu]

Attachment: enable "new folder" feature for pop accounts

POP folder is free now. :)

Comment 23

[Calvin Liu]

BTW, the patches here have passed the review tool test. You know that "JST
Review Simulacrum"? A perl script to check the grammar that patch use. I don't
know why you couldn't add the patch. What's the error message you got?

Comment 24

[David :Bienvenu]

why not just check for imap instead of checking for pop3 and local?

Comment 25

[Calvin Liu]

At first sight, for backward compatibility. We know that the menu should be
enabled for pop3 and local and disabled for imap, but don't know if there will
be some other new protocols and what's the principle for them. (Can we suppose
that the new protocols are better then imap and also need disable this menu?)

But now I also think it's not necessary to worry about this. Actually, it's
better to ask end users if they need to go online when create a folder offline.
If their answer is "yes" then we go online and do it, otherwise we can just
simply cancel this operation.

Anyway, I can change the patch if you think it should be.

Comment 26

[David :Bienvenu]

Calvin, what if we add a new protocol and it doesn't need to be disabled? Then
we'd need to add a check for it...since the bug is to disable this for imap, we
should be checking for imap, I think. 
Also, eventually, we'll support creating imap folders offline (it almost works
in the backend) so it will be easier to find and remove this check if it refers
to imap.

Comment 27

[Calvin Liu]

Something is omitted. Should we also enable the "File"->"New"->"Folder" for 
pop3 and local folder?

Comment 28

[David :Bienvenu]

we should just disable it for imap.

Comment 29

[Calvin Liu]

Attachment: new patch

hope this time it's ok.

Comment 30

[Calvin Liu]

Attachment: kill this bug!

last patch doesn't work for I mis-spelling a "new" to "New" and some problems
can't pass the famous jst-review tool.

Comment 31

[Calvin Liu]

Hi, please review my patch. Thanks.

Comment 32

[Sean Su]

Comment on attachment 85427 [details] [diff] [review]
kill this bug!

off line POP3's New Folder menus is working as expected, as is IMAP's New
Folder menus.

r=ssu

Comment 33

[David :Bienvenu]

the two places that this code appears can be rewritten more cleanly:
+  var aFolder = GetFolderAttribute(folderTree, folderResource, "ServerType");
+  var isIMAPFolder = aFolder == "imap";

should be
  var isIMAPFolder = GetFolderAttribute(folderTree, folderResource,
"ServerType") == "imap";

(aFolder is not a good variable name - it's really folderServerType, but if you
get rid of the variable, you don't need to fix the name)
and

+  if (isIMAPFolder)
+    EnableMenuItem("folderPaneContext-new", !ioService.offline);
+  else
+    EnableMenuItem("folderPaneContext-new", true);

should be
  EnableMenuItem("folderPaneContext-new", !isIMAPFolder || !ioService.offline);

+// Change "New Folder..." menu accroding to the context
should be "according"

Comment 34

[Calvin Liu]

Attachment: new patch

Yes, aFolder is used to debug, should be removed. I've modified my patch. Could
you give a r= and sr= again?

Comment 35

[David :Bienvenu]

it doesn't look to me like you addressed either of my comments. Did you really
attach a new patch?

Comment 36

[Calvin Liu]

Attachment: updated patch

i only modified mailWindowOverlay.js but forgot to change mailContextMenus.js.
now it's ok.

Comment 37

[David :Bienvenu]

Comment on attachment 91923 [details] [diff] [review]
updated patch

ok, thx, sr=bienvenu

Comment 38

[Asa Dotzler [:asa]]

Comment on attachment 91923 [details] [diff] [review]
updated patch

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

Comment 39

[Antonio.Xu]

patch checked into trunk

Comment 40

[Antonio.Xu]

patch checked into NETSCAPE_7_0_OEM_BRANCH 

Comment 41

[grylchan]

Using commercial trunk
2002-09-19-08-trunk/ NT 4.0
2002-09-20-08-trunk/ Mac 10.1.5, mac 9.2.2
2002-09-20-08-trunk/ linux 2.2, red hat 7.0

This is fixed for the follwing:
 -Main File menu in messenger
 -context menu for any folder
 -Right click, New folder
 -context menu from a mail account in the folder pane

New Folder/New Subfolder is disabled for imap accounts while
offline. It is enabled for pop accounts & Local folders 
when you are offline.

It is not fixed at:
 - in the file menu in both a downloaded and non-downloaded stand
  alone message window

Filed a new bug, bug 169976. Don't know if Calvin wants to
fix this or not.

Didn't test OEM builds. Assume Calvin or Antonio will do this.

marking verified.