Skip to content

Move-folder modal: self-move guard is dead code and leaves moving stuck #13534

Description

@kesselb

Summary

Flagged by Claude during a code review.
Needs validation.

Problem

onMove() sets this.moving = true, then guards the whole operation with this.mailbox.id !== this.destMailboxId. Two defects:

  1. The comparison can never be true. Mailbox::jsonSerialize (lib/Db/Mailbox.php:176-177) exposes id as base64_encode($name), while destMailboxId is always the numeric databaseId or undefined. The intended "moving a folder into itself is a no-op" check therefore never fires.
  2. The false branch resets nothing — no moving = false, no $emit('close'). Only the try/finally inside the if does that.

Consequences

  • Selecting root for a folder that already sits at root issues a rename to its own name; the resulting IMAP error is only logged, so the user sees a silent no-op instead of the guard.
  • Correcting the comparison to databaseId without also fixing the state handling would hang the dialog: since fix: minor adjustment in moving messages, threads and folders #13431 made the handler's promise the one NcDialogButton awaits, the primary button would stay disabled forever.

Suggested fix

Compare this.mailbox.databaseId against this.destMailboxId, and return before setting moving = true when they match (or hoist the finally cleanup to cover the early exit).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions