GNOME Bugzilla – Bug 476919
eog save menu is always active
Last modified: 2007-10-29 20:27:09 UTC
This bug has been filled here: https://bugs.launchpad.net/ubuntu/+source/eog/+bug/139602 " Binary package hint: eog steps to reproduce it: 1- run eog. 2- open any image file. 3- without doing any change, go to file menu and save. 4- iterate in the third point. NOTE: this doesn't actually save the image, the point is that instead of being greyed, the menu is always active" Thanks.
Thank you for taking the time to report this bug and helping to make Eye of GNOME better. I'll confirm this.
I think we can include the Undo menu item here too. It is just like the Save item only usable if the image is actually changed.
Created attachment 97003 [details] [review] proposed fix Attached patch fixes this by connecting to the "changed" signal emitted by the image to determine if we can save or not.
Thanks. Just had a quick look (I'm in a hurry) and it seems to work quite good so far. Two things I noticed: 1. When I return the image to its unmodified state (by using Undo) the two buttons stay active. 2. You initially inactivate the actions, but I think it is possible that the image is already modified during loading due to the EXIF autorotation. We at least need to be able to save the image then (I guess we can skip undo here as I'm not sure it even works in that situation). The eog_image_is_modified() function could be of help in both cases.
(In reply to comment #4) > 2. You initially inactivate the actions, but I think it is possible that the > image is already modified during loading due to the EXIF autorotation. We at > least need to be able to save the image then (I guess we can skip undo here as > I'm not sure it even works in that situation). EXIF auto-orientation should be considerer an image modification. It's part of the image. Therefore, you should not be able to save the image if it was auto-rotated.
Created attachment 97099 [details] [review] proposed fix v2 Attached patch sets sensitivity in the callback of load, save and transform jobs, and implements suggestions by Felix and Lucas. Also, I made "Save" insensitive right after saving (becomes sensitive when a change is made).
Cosimo, thanks for the patch! The patch is quite ok. The only thing I don't like is that it makes the EOG_CONF_DESKTOP_CAN_SAVE key be read each time you apply any transformation. You could make save_disabled an EogWindow private attribute and cache it for future usages. As a bonus, you could keep track of changes in this key and change the UI accordingly.
Created attachment 97284 [details] [review] proposed fix v3 Thanks Lucas, I updated the patch according to your last comments.
(In reply to comment #5) > > EXIF auto-orientation should be considerer an image modification. It's part of > the image. Therefore, you should not be able to save the image if it was > auto-rotated. > This makes it impossible (well, nearly) to let the image data reflect the rotation (and reset the EXIF tag). This might be needed in cases where EXIF rotation is unsupported, for example if you want to burn your photos on DVD to show them on your TV screen (not every DVD player supports EXIF data). We tell the user in the manual that he needs to save the image to make the autorotation permanent, btw.
Fixed some remaining issues in the patch and commited in trunk and gnome-2-20 branch. Thanks Cosimo! 2007-10-29 Lucas Rocha <lucasr@gnome.org> * src/eog-window.c (eog_window_can_save_changed_cb, update_action_groups_state, eog_job_load_cb, eog_job_transform_cb), eog_job_save_cb, eog_window_construct_ui, eog_window_init): correctly update save actions according to image modification state and desktop save to disk lockdown key. Fixes bug #476919 (Cosimo Cecchi).