Skip to content

fix(crash): unhook signal to avoid double widget destruction - #22310

Closed
Geobert wants to merge 2 commits into
darktable-org:masterfrom
Geobert:fix-crash-tag-floating-window
Closed

Geobert wants to merge 2 commits into
darktable-org:masterfrom
Geobert:fix-crash-tag-floating-window

Conversation

@Geobert

@Geobert Geobert commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

When hitting Return or Escape when completion list is visible, both _lib_tagging_tag_destroy hooked by

g_signal_connect(entry, "focus-out-event",

and
gtk_widget_destroy(d->floating_tag_window);

boith got called and destroys twice the floating window

unhook the signal on Return and Escape to avoid that

  • fix in the flake.nix to allow build on NixOS unstable

  • fix in the build.sh to be compatible with more Linux distrib (NixOS)

Referenced issue

Fixes #20919

Checklist

  • I have read CONTRIBUTING.md and the
    coding style.
  • The pull request is one logical change, and every commit compiles on its
    own.
  • I ran the relevant tests: unit tests, src/tests/integration/ where the
    pixelpipe is touched, or darktable-cli as a headless smoke test.
  • New user-visible strings use _(), new preferences are registered in
    data/darktableconfig.xml.in.
  • A RELEASE_NOTES.md entry was added.

Test instructions

Was crashing with the procedure described in #20919 and not crashing anymore

AI assistance

None

@Geobert
Geobert force-pushed the fix-crash-tag-floating-window branch 2 times, most recently from 44cd496 to 7955c02 Compare September 17, 2026 09:11
@Geobert

Geobert commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

I can’t see what’s wrong in the checks that didn’t pass :-/

@zisoft

zisoft commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator
/Users/runner/work/darktable/darktable/src/src/libs/tagging.c:3862:7: fatal error: label followed by a declaration is a C23 extension [-Wc23-extensions]
 3862 |       GtkWidget *entry = dt_gui_get_widget(controller);
      |       ^
1 error generated.

You cannot declare a local variable inside the case block of a switch statement.
Move the variable declaration GtkWidget *entry = NULL; before the switch.

@Geobert
Geobert force-pushed the fix-crash-tag-floating-window branch from 7955c02 to 4744350 Compare September 17, 2026 13:22
@Geobert

Geobert commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! I did a search on error and it didn’t show up, I guess I need to download the whole log

@zisoft

zisoft commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

search for error: (with the colon) in the logs.

Comment thread RELEASE_NOTES.md Outdated
@Geobert
Geobert force-pushed the fix-crash-tag-floating-window branch from 4744350 to 74dc5a5 Compare September 17, 2026 16:50

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please separate the different changes on separate commits. The PR is about signal and this is only covered by changes in tagging.c.

@TurboGit TurboGit added this to the 5.8 milestone Sep 18, 2026
@TurboGit TurboGit added bugfix pull request fixing a bug scope: codebase making darktable source code easier to manage labels Sep 18, 2026
When hitting Return or Escape when completion list is visible, both `_lib_tagging_tag_destroy` hooked by https://github.com/darktable-org/darktable/blob/733a44142ae373a85f3dc1b342d92de725d8caa3/src/libs/tagging.c#L3973
 and
https://github.com/darktable-org/darktable/blob/733a44142ae373a85f3dc1b342d92de725d8caa3/src/libs/tagging.c#L3876

boith got called and destroys twice the floating window

unhook the signal on Return and Escape to avoid that
@Geobert
Geobert force-pushed the fix-crash-tag-floating-window branch from 74dc5a5 to accfaf9 Compare September 18, 2026 20:57
@Geobert
Geobert requested a review from TurboGit September 18, 2026 21:06

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@TurboGit

Copy link
Copy Markdown
Member

@Geobert : I'll see if I can rebase from GitHub, but please remember to never merge master into your topic branch. The policy on Darktable is to rebase when needed.

@Geobert

Geobert commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

I though I did a rebase… After all these years as a dev, I still suck at git x)

@TurboGit

Copy link
Copy Markdown
Member

Manually merged with conflict resolution.

@TurboGit TurboGit closed this Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug scope: codebase making darktable source code easier to manage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash: Quick tagging after modifiying description meta data

3 participants