Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #697: a crash in the Watermark plugin if screenshot name includes spaces #698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ sub apply_effect {

my $command2 = "composite " . $tmpfilename .
" -gravity " . $gravity_combo->get_active_text .
" " . $filename . " " . $tmpfilename2;
Copy link
Member

Choose a reason for hiding this comment

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

better not to use backtick at all, but to use some other ways to run subprocess. Or at least escape filesnames and such.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, with a system call it resulted in an error which I couldn't understand (here is my question about it: https://stackoverflow.com/questions/68628648/the-and-comma-operators-in-a-perl-system-call).

What else would you propose instead of backticks?

Copy link
Member

Choose a reason for hiding this comment

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

If I just revert your previous change, the system call works fine for me. What was the issue?

Copy link
Member Author

@Photon89 Photon89 Sep 7, 2024

Choose a reason for hiding this comment

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

It crashed when clicking on "refresh" or "apply" three years ago, see #379.

I can actually still reproduce it. The plugin window just disappears and the screenshot stays untouched.

Copy link
Member

Choose a reason for hiding this comment

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

can you show logs of this?

it works perfectly for me, if I revert your old commit

Copy link
Member

Choose a reason for hiding this comment

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

Please try #702

Copy link
Member Author

Choose a reason for hiding this comment

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

Still crashes as in #379, unfortunately. Note for later, perl-ipc-run3 as new dependency required if we want to pursue this variant.

Copy link
Member

Choose a reason for hiding this comment

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

But is the log any better there why it crashes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, it's still the same...

Copy link
Member

Choose a reason for hiding this comment

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

well, I don't know how to debug this further for you.

you can merge this PR, of course, but don't be surprised when the next issue opened will be "crash if the name includes the quote character" or something like that

can you try to write a small script (without any shutter or even gtk) which just runs these 2 commands via backticks / via run3, and see what it says?

" " . "\"$filename\"" . " " . $tmpfilename2;
my $output2 = `$command2`;
}

Expand Down
Loading