After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 772621 - Empty comments in annotations server no purpose but get created at the first click
Empty comments in annotations server no purpose but get created at the first ...
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: pdf annotations
3.24.x
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-08 18:22 UTC by Gordian Edenhofer
Modified: 2017-12-25 08:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libview: Auto-remove empty annotations (2.40 KB, patch)
2016-10-09 15:47 UTC, Felipe Borges
none Details | Review
Screenshot showing empty tooltip for annotation (148.10 KB, image/png)
2017-12-10 12:24 UTC, Nelson Benitez
  Details
ev-view: don't show tooltips for empty annotations (1.03 KB, patch)
2017-12-10 12:26 UTC, Nelson Benitez
none Details | Review
ev-view: don't show tooltips for empty annotations (1.02 KB, patch)
2017-12-23 09:29 UTC, Nelson Benitez
committed Details | Review

Description Gordian Edenhofer 2016-10-08 18:22:01 UTC
When clicking on an annotation a comment section pops up. However even if left empty and closed immediately an empty comment gets created and is visible when hovering over the annotation.
Comment 1 Felipe Borges 2016-10-09 15:47:58 UTC
Created attachment 337280 [details] [review]
libview: Auto-remove empty annotations
Comment 2 José Aliste 2016-10-11 10:28:55 UTC
Review of attachment 337280 [details] [review]:

I don't think this is what we want. removing annotations shouldb e some explicit user interaction. I think what we want is not to show an hover over annotations without content.
Comment 3 Gordian Edenhofer 2016-10-11 10:38:28 UTC
Adding empty annotations which do not get displayed is even more awkward for the user since the document gets changed without an apparent reason. I think removing the comment from the annotations if it is empty is the right choice.
Comment 4 Philipp Raich 2017-07-26 14:18:55 UTC
I'm also not sure this would work for everyone. The description of the proposed patch would suggest that an annotation that has no comment will be deleted? People make lots of empty annotations (text highlight) - would these be removed?

I'm not entirely sure how the backend handles annotations and how various readers create them (I am also aware that acrobat handles comments slightly differently to evince/poppler, i.e. comment threads), so correct me if I am wrong. I assume that annotations with empty comments at least in poppler are a thing and should be OK, might even not be avoidable depending on the implementation. Is it possible to have annotations with comments with poppler?

Nevertheless, I agree that an empty comment does not need to be displayed. Not displaying these comments while leaving the annotation itself alone would be an option then?
Comment 5 Nelson Benitez 2017-12-10 12:24:25 UTC
Created attachment 365318 [details]
Screenshot showing empty tooltip for annotation

Yes, "highlight" annotations without comments can be quite common, and there is no point in showing an empty tooltip for them.

I will attach a patch to not show these empty tooltips.
Comment 6 Nelson Benitez 2017-12-10 12:26:25 UTC
Created attachment 365319 [details] [review]
ev-view: don't show tooltips for empty annotations

Empty annotations can be quite common, eg. when
adding highlight annotations where the user doesn't
enter a comment.

So don't show show empty tooltips as they are useless.
Comment 7 Gordian Edenhofer 2017-12-10 15:05:38 UTC
Thanks for you work towards fixing this issue!
Comment 8 Carlos Garcia Campos 2017-12-12 13:10:11 UTC
Review of attachment 365319 [details] [review]:

::: libview/ev-view.c
@@ +4754,3 @@
 
+		contents = ev_annotation_get_contents (annot);
+		if (contents && strcmp (contents, "")) {

I don't think we need to use strcmp for this, you could simply check *contents (or contents[0]) != '\0'. I think it's easier to read than strcmp.
Comment 9 Nelson Benitez 2017-12-23 09:29:51 UTC
Created attachment 365896 [details] [review]
ev-view: don't show tooltips for empty annotations

Empty annotations can be quite common, eg. when
adding highlight annotations where the user doesn't
enter a comment.

So don't show show empty tooltips as they are useless.

https://bugzilla.gnome.org/show_bug.cgi?id=772621

------

Updated patch, indeed checking for NULL is better.
Comment 10 Nelson Benitez 2017-12-25 08:32:31 UTC
Comment on attachment 365896 [details] [review]
ev-view: don't show tooltips for empty annotations

Pushed as commit de9657875d6da3a14ec3ac852079cdcb00d74f6f

Thanks!
Comment 11 Nelson Benitez 2017-12-25 08:35:20 UTC
Comment on attachment 365896 [details] [review]
ev-view: don't show tooltips for empty annotations

Pushed as commit de9657875d6da3a14ec3ac852079cdcb00d74f6f

Thanks!