Signal-iOS icon indicating copy to clipboard operation
Signal-iOS copied to clipboard

Disappearing Bottom Controls in ImageEditorCropViewController

Open ChronicLynx opened this issue 10 months ago • 1 comments

  • [x] I have searched open and closed issues for duplicates
  • [x] I am submitting a bug report for existing functionality that does not work as intended
  • [x] This isn't a feature request or a discussion topic

Bug description

There appears to be a very rare case where the bottom controls when cropping an image can entirely disappear, leaving you unable to get out of the crop tool without force closing the application. Behavior seems almost like a race condition in that it only happens very rarely and is difficult to reliably reproduce. Somehow the buttons are getting stuck in the hidden state I think, and is related to the transitionUI function.

I've also had this happen with the markup tool which appears to use the same animation methods for the bottom row of buttons.

Steps to reproduce

  • Open and close the crop editor and spam inputs. Really tough to get it to happen. It will naturally happen to me about once a day or every other day.

Actual result: Navigation controls will sometimes be absent, leaving the user unable to exit the crop tool.

Expected result: Navigation controls should ALWAYS be present when in the crop tool. Disabling animations seems to fix this, but I'd like to come up with a solution that preserves the animations and prevents the potential race condition.

Screenshots

Device info

Device: iPhone 16 Pro Max

iOS version: 18.3.1 (and previous releases)

Signal version: 7.47 (625)

Link to debug log

https://debuglogs.org/ios/7.47.0/e321bc0df8a08148136ed43d6dcc607a27ca9f8e02c3db11681d37fd8cc74dd8.zip

This error stands out to me in relation to the issue: 2025/02/20 00:54:25:091 ERR❤️ [AttachmentPrepViewController.swift:221 presentFullScreen(viewController:)]: Already has presented view controller. [<SignalUI.ImageEditorCropViewController: 0x127aef700>]

ChronicLynx avatar Feb 20 '25 01:02 ChronicLynx

@sashaweiss-signal

I assume 7.48 had those view controller presentation changes, as I have not encountered the Already has presented view controller error again. I did have one scenario today on 7.48 where the buttons disappeared after tapping cancel but the view did not dismiss. I'm thinking there could also be a scenario where the guard statement runs that return and never dismisses the view, thus leaving you stuck like I was.

ImageEditorCropViewController

@objc
private func didTapCancel() {
    transitionUI(toState: .initial, animated: true) { finished in
        guard finished else { return }
        self.dismiss(animated: false)
    }
}

@objc
private func didTapDone() {
    model.replace(transform: transform)
    transitionUI(toState: .initial, animated: true) { finished in
        guard finished else { return }
        self.dismiss(animated: false)
    }
}

Perhaps it should just be this to make it more robust? It doesn't seem ideal that there's a path for the navigation buttons to be gone and also not dismiss the view.

@objc
private func didTapCancel() {
    transitionUI(toState: .initial, animated: true) { _ in
        self.dismiss(animated: false)
    }
}

@objc
private func didTapDone() {
    model.replace(transform: transform)
    transitionUI(toState: .initial, animated: true) { _ in
        self.dismiss(animated: false)
    }
}

Let me know what you think.

ChronicLynx avatar Feb 26 '25 19:02 ChronicLynx

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jun 06 '25 12:06 github-actions[bot]

This issue has been closed due to inactivity.

github-actions[bot] avatar Jun 13 '25 12:06 github-actions[bot]