GNOME Bugzilla – Bug 725571
unable to change the color and opacity of annotation window
Last modified: 2014-06-22 22:21:11 UTC
It seems that we can't change the annotation-window's color and opacity by doing changes in annotation-properties, although we change both(color and opacity) for the annotation-icon.
Created attachment 278754 [details] [review] Connecting to the right signal so that the color of the annotation window is changed I looked into the opacity issue as well. Although it is possible to change the opacity of a widget, I think this option should not be available for annotation text. I say this because poppler does not support opacity for this type of annotation (only annotation markup has opacity). This means that the information could not be saved to the pdf and once the user opens the document again, the window will still be opaque. What do you think? I can create a patch removing the opacity option from the properties window for annotation text if you agree.
Review of attachment 278754 [details] [review]: Thanks for the patch, it looks good, but I think the actual fix would be to emit the notify::color signal when ev_annotation_set_rgba() is called. Note that the callback ev_annotation_window_color_changed() is using color, not rgba, so it's not consistent. You should also explain a bit the bug you are fixing in the commit message, the real problem is that notify::color is not emitted when the color is changed with the rgba interface.
(In reply to comment #1) > Created an attachment (id=278754) [details] [review] > Connecting to the right signal so that the color of the annotation window is > changed > > I looked into the opacity issue as well. Although it is possible to change the > opacity of a widget, I think this option should not be available for annotation > text. I say this because poppler does not support opacity for this type of > annotation (only annotation markup has opacity). This means that the > information could not be saved to the pdf and once the user opens the document > again, the window will still be opaque. What do you think? I can create a patch > removing the opacity option from the properties window for annotation text if > you agree. MarkupAnnotation is not a class but an interface, and text annotations are also markup annotations and they implement the interface.
(In reply to comment #2) > Review of attachment 278754 [details] [review]: > > Thanks for the patch, it looks good, but I think the actual fix would be to > emit the notify::color signal when ev_annotation_set_rgba() is called. Note > that the callback ev_annotation_window_color_changed() is using color, not > rgba, so it's not consistent. You should also explain a bit the bug you are > fixing in the commit message, the real problem is that notify::color is not > emitted when the color is changed with the rgba interface. Thank you for the quick feedback! Since ev_annotation_get_color is deprecated, I could also change everything to rgba and it would be consistent, is this ok?
Yes, but that would be a different bug. So, I would first fix this bug, by emitting notify::color in ev_annotation_set_rgba() and then file a new bug report to use rgba instead of color everywhere.
Created attachment 278768 [details] [review] Emitting both signals in set_rgba Ok, here's the patch. I will create a patch for changing to rgba later and in the meantime I am looking into the opacity issue. Thanks!
Review of attachment 278768 [details] [review]: Thanks!
Created attachment 278835 [details] [review] Fix the opacity issue For the opacity problem the only thing missing was to connect the "opacity" signal to the annotation window and implement the functions that actually change the property for the widgets used.
Review of attachment 278835 [details] [review]: LGTM, thanks!
Review of attachment 278768 [details] [review]: Pushed.
Review of attachment 278835 [details] [review]: Pushed.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.