Comment 128 for bug 214366

Comment on attachment 8371072
WIP patch 3

Review of attachment 8371072:
-----------------------------------------------------------------

Nice test.

::: mailnews/local/src/nsLocalMailFolder.cpp
@@ +1723,5 @@
> return NS_MSG_ERROR_COPY_FOLDER_ABORTED;
> }
> }
> +
> + nsAutoString newFolderName(EmptyString());

Shouldn't need the EmptyString() initializer; the constructor makes an empty string by default.

@@ +1726,5 @@
> +
> + nsAutoString newFolderName(EmptyString());
> + nsAutoString folderName;
> + rv = srcFolder->GetName(folderName);
> + NS_ENSURE_SUCCESS(rv, rv);

Mozilla core code is deprecating NS_ENSURE_SUCCESS, because the macro hides control flow - see https://groups.google.com/d/topic/mozilla.dev.platform/1clMLuuhtWQ/discussion . Use

if (NS_WARN_IF(NS_FAILED(rv))) {
  return rv;
}

if the condition deserves a warning message (as this one does)

::: mailnews/local/test/unit/test_nsIMsgLocalMailFolder.js
@@ +50,5 @@
> + do_check_eq(trash.numSubFolders, 3);
> + // The folder should be automatically renamed as the same name already is in Trash
> + // but the subfolder should be untouched.
> + let folderDeleted3 = trash.getChildNamed("folder(3)");
> + let subfolderDeleted = folderDeleted3.getChildNamed("subfolder");

do_check_true(trash.containsChildNamed("folder(3)");
do_check_true(trash.getChildNamed("folder(3)").containsChildNamed("subfolder"));