Skip to content

Conversation

@bhennion
Copy link
Collaborator

@bhennion bhennion commented Dec 11, 2025

Fix #6240

This fixes the crash, but does not really fix the underlying issue with the automatic page addition feature.

The crash was caused by the following path. As the user drags the selection downwards, the EditSelection::edgePanHandler() callback gets called. It triggers a scrolling (Layout::scrollRelative()). This scrolling in turn triggers a call to Layout::maybeAddPage(), with then adds a page.
When adding a page, we always clear selections and text edition (see PageBackgroundController::insertNewPage()), so the EditSelection instance which triggered all that gets destroyed... We then get the SegFault when the EditSelection instance gets accessed after deletion.

Now, this PR fixes the crash, BUT the selected items still often disappear (they are in fact added to a page, but outside its bound and thus get deleted...).
A proper fix would need to have the automatic page addition feature made aware of existing selections, and ensuring that the content stays selected. That would be some work - the code around this feature is a bit scattered and some refactoring would be required.

Edit: Stroke out the outdated description. See post below.

@bhennion bhennion requested a review from rolandlo December 11, 2025 14:35
@bhennion
Copy link
Collaborator Author

I implemented a much better fix. It wasn't as complicated as I thought.

So now:

  1. It doesn't crash anymore
  2. The selection stays selected when pages are added automatically (upon scrolling, typically).
  3. The automated page insertion is disabled if the inserted page would be a image page. Otherwise (e.g. on master), the dialog would pop up, asking the user to pick an image...

@bhennion bhennion added this to the v1.3.1 milestone Dec 11, 2025
Copy link
Member

@rolandlo rolandlo left a comment

Choose a reason for hiding this comment

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

LGTM! I only have one minor comment about an include.

@bhennion bhennion changed the base branch from master to release-1.3 December 18, 2025 13:47
@bhennion
Copy link
Collaborator Author

Thanks for the review!
I just realized the PR had target the master branch. Are you ok with me merging this to the release-1.3 branch?

@rolandlo
Copy link
Member

Thanks for the review! I just realized the PR had target the master branch. Are you ok with me merging this to the release-1.3 branch?

Yes, sure. I didn't notice that.

@bhennion bhennion merged commit b6cd107 into xournalpp:release-1.3 Dec 18, 2025
9 checks passed
@bhennion bhennion deleted the pr/fixsel branch December 18, 2025 14:00
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 create new last page by dragging a selection

2 participants