(In reply to n...@parkwaycc.co.uk from comment #122)
> Comment on attachment 8374439
> WIP patch 4
> 
> >+    bool containsChild = true;
> >+    uint32_t i;
> >+    for (i = 1; ; i++) {
> >+      newFolderName.Assign(folderName);
> >+      if (i > 1) {
> >+        // This could be localizable but Toolkit is fine without it, see
> >+        // mozilla/toolkit/content/contentAreaUtils.js::uniqueFile()
> >+        newFolderName.Append('(');
> >+        newFolderName.AppendInt(i);
> >+        newFolderName.Append(')');
> >+      }
> >+      rv = ContainsChildNamed(newFolderName, &containsChild);
> >+      if (NS_WARN_IF(NS_FAILED(rv))) {
> >+        return rv;
> >+      }
> >+      if (!containsChild)
> >+        break;
> >+    }
> Did you mean for (i = 1; containsChild; i++) ? Note that if you do this then
> i ends the loop being at least 2, so you'd need to change the subsequent
> check. Alternatively, you would have to rewrite the loop something like this
> (sorry if this is what the code did in one of the earlier patches):
> 
> uint32_t i = 1;
> newFolderName.Assign(folderName);
> for (;;) {
>   bool containsChild;
>   rv = ContainsChildNamed(newFolderName, &containsChild);
>   NS_ENSURE_SUCCESS(rv, rv);
>   if (!containsChild)
>     break;
> 
>   i++;
>   newFolderName.Assign(folderName);
>   newFolderName.Append('(');
>   newFolderName.AppendInt(i);
>   newFolderName.Append(')');
> }
Yes, this is what the previous version did, just with a 'while'.
Is this issue really that important that we make tens of comments in such a 
long thread? It is not a perf critical path.

> >+  try {
> >+    root.copyFolderLocal(folderDeleted3, true, null, null);
> >+    do_throw("copyFolderLocal() should have failed here due to user 
> >prompt!");
> I'm not sure why you're trying to throw from inside this try block, but I
> don't know enough about writing tests to know whether there's a better way.
I expect .copyFolderLocal to throw and be catched by my catch. But if it does 
not throw (wrong behaviour), the next throw is not catched and fails the test.

> 
> >+  } catch (e) {
> >+    do_check_eq(e.result, parseInt("0x8055001a", 16));
> Why not just write 0x8055001a?
Because e.result returns the code in decimal. Is there other method of 'e' I 
can use?

> >+confirmDuplicateFolderRename=A folder with that name already exists under 
> >folder '%1$S'. Would you like to copy the folder under a new name of '%2$S'?
> A subfolder with that name already exists in the folder '%1$S'. Would you
> like to move the folder using the new name of '%2$S'?

Irving proposed some other wording with more arguments. So what should I
do now?

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/214366

Title:
  Can't delete a folder if Trash already contains a folder of the same
  name

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/214366/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to