Skip to content

fix: add missing errorText widget to change_passphrase.ui#2467

Open
Fikri-20 wants to merge 4 commits intoborgbase:masterfrom
Fikri-20:fix/change-passphrase-error-text
Open

fix: add missing errorText widget to change_passphrase.ui#2467
Fikri-20 wants to merge 4 commits intoborgbase:masterfrom
Fikri-20:fix/change-passphrase-error-text

Conversation

@Fikri-20
Copy link
Copy Markdown

Fixes #2454

What

ChangeBorgPassphraseWindow._set_status() calls self.errorText.setText(), but the errorText QLabel widget was missing from change_passphrase.ui. This caused an AttributeError crash whenever an error needed to be displayed — for example, after changing the keyring provider and attempting to update a repo passphrase.

Fix

Added the missing errorText QLabel to change_passphrase.ui, using the same widget definition already present in repo_add.ui.

Test

Added a regression test test_change_passphrase_window_error_text in tests/unit/test_repo.py that instantiates the window and verifies _set_status() sets and clears the error text without crashing.

_set_status() in ChangeBorgPassphraseWindow called self.errorText.setText()
but the widget was absent from the UI file, causing an AttributeError crash
when displaying errors after a keyring change.

Adds the errorText QLabel (matching the pattern used in repo_add.ui) and a
regression test that verifies _set_status() works without crashing.

Fixes borgbase#2454
@Fikri-20
Copy link
Copy Markdown
Author

hey @m3nu , could you please review the PR and tell me if I need to tweak anything else?

@Fikri-20
Copy link
Copy Markdown
Author

Hi @m3nu, I'm applying for GSoC 2026 and have these PR ready for review. Could you please take a look?

m3nu
m3nu previously approved these changes Mar 25, 2026
Copy link
Copy Markdown
Contributor

@m3nu m3nu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good — the fix is correct and the test covers the regression.

Minor nits (non-blocking):

  • scaledContents on the errorText QLabel is a no-op (only applies to pixmap content). It's fine since it's copied from repo_add.ui for consistency, but could be dropped in a future cleanup.

  • Worth noting: the patch originated from the issue reporter in #2454. Good on you for formalizing it as a PR with a regression test.

@Fikri-20
Copy link
Copy Markdown
Author

Hi @m3nu
Is it good to merge PR now?

@Fikri-20
Copy link
Copy Markdown
Author

Hey @m3nu, addressed your nit — dropped the scaledContents property from the errorText QLabel since it's a no-op for text labels. Branch is updated. Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when trying to display an error after changing keyring

2 participants