search

mate-desktop / mate-notification-daemon

Daemon to display passive pop-up notifications
https://mate-desktop.org
GNU General Public License v2.0
30 stars 26 forks source link

Fix #117, unescape URI-encoded paths for icon files #121

Closed AlfioEmanueleFresta closed 7 years ago

AlfioEmanueleFresta commented 7 years ago

Fixes issue #117, which caused file:// URIs containing correctly-escaped special characters, such as spaces (%20), not to be resolved.

raveit65 commented 7 years ago

How can this be tested?

raveit65 commented 7 years ago

Btw. notify-send /home/rave/Bilder/%20image.jpg displays me the link. or notify-send -i /home/rave/Bilder/%20image.jpg test displays a test notification with an icon inside. @AlfioEmanueleFresta What is wrong without your PR?

raveit65 commented 7 years ago

hmm, notify-send -i /home/rave/Bilder/my image.jpg test doesn't display the image Here image is used for header and test is the body. But with your PR i see no different. So, please provide some test instructions. Of course this one works well. notify-send -i /home/rave/Bilder/my\ image.jpg test

AlfioEmanueleFresta commented 7 years ago

@raveit65, in normal paths, %20 should not be replaced with a space character. This is because the percentage sign is allowed in file names, and you may legitimately have a filename with %20 in it. In your case, you should be able to normally load an image named %20image.jpg, but if you rename your image to have a leading space character in the file name, e.g,

/home/rave/Bilder/ image.jpg

you should not be able to access it by referring it as,

/home/rave/Bilder/%20image.jpg

However, file:// URIs may have URL-encoded characters (as per RFC 1738), and these need to be decoded. In your case, you should be able to access your image with a whitespace in the filename,

/home/rave/Bilder/ image.jpg

by using the following file URI,

file:///home/rave/Bilder/%20image.jpg

In conclusion, to test this PR, you may create an image path with spaces in it, e.g,

/home/rave/path with spaces/test image.jpg

then,

notify-send -i "file:///home/rave/path%20with%20spaces/test%20image.jpg" "Summary"

should correctly load the image.

I hope this clarifies this issue.

raveit65 commented 7 years ago

Yeap, that works without your PR

notify-send -i "file:///home/rave/Bilder/path%20with%20spaces/my%20image.jpg" "Summary"

doesn't display the image. With your PR it works.

raveit65 commented 7 years ago

merged and reworded commit message a bit. https://github.com/mate-desktop/mate-notification-daemon/commit/0a84eac2c5115d6a21e98e4797fbf6407cf5712b The hint 

Fix #117

belongs to commit body and not in header ;-)

raveit65 commented 7 years ago

...thank you.