Bug 319442 - Initial contents of Inline Note annotation discarded
Summary: Initial contents of Inline Note annotation discarded
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 0.16.60
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-07 00:12 UTC by Jon Mease
Modified: 2013-05-14 07:46 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Mease 2013-05-07 00:12:17 UTC
The initial contents of an Inline Note annotation are not preserved when the note 's contents are later edited.

Reproducible: Always

Steps to Reproduce:
1. Create an inline annotation
2. Enter some initial text in the popup window (The inline note will be created with these contents).
3. Double click the newly created inline note
4. Observe what text the popup annotation window is populated with
Actual Results:  
The popup window's text is empty

Expected Results:  
The popup window should be populated with the initial text entered in step (2) above

It looks like the problem is that when the inline note annotation is created the initial text is set using Annotation::setInplaceText (Reference 1), however the annotation popup window is populated using the annotation's contents (Reference 2).  When the undo/redo functionality was added the intention was to do away with the distinction between an annotation's contents and inplaceText (See discussion in https://git.reviewboard.kde.org/r/107442/), but this case was not addressed.

Proposal:  Remove m_inplaceText member from annotationPrivate but remap calls to Annotation::(set)inPlaceText to calls to Annotation::(set)contents for backwards compatibility.  While investigating I noticed that the function in Reference 3 should also be cleaned up when this is fixed.

References:
1) PickPointEngine::end in pageviewannotator.cpp
2) AnnotWindow::AnnotWindow
3) DocumentPrivate::performSetAnnotationContents in document_p.cpp
Comment 1 Jon Mease 2013-05-07 00:15:02 UTC
Fabio, what do you think of the proposal above?
Comment 2 Fabio D'Urso 2013-05-07 00:26:02 UTC
(In reply to comment #0)
> Proposal:  Remove m_inplaceText member from annotationPrivate but remap
> calls to Annotation::(set)inPlaceText to calls to Annotation::(set)contents
> for backwards compatibility.
Feel free to remove it, there's no one using it apart from ourselves afaik.

Haven't read the rest. I'll do it tomorrow (it's night here.... yawn!)  :)
Comment 3 Fabio D'Urso 2013-05-07 23:02:00 UTC
I agree with everything you said.
I'm for removing Annotation::(set)inPlaceText at all, in order to simplify the API.
Comment 4 Jon Mease 2013-05-11 19:51:29 UTC
Thanks for your thoughts Fabio. 

Proposed fix at https://git.reviewboard.kde.org/r/110391/

I removed the inplaceText and window text attributes (along with getters and setters) and replaced all usages with (get/set)contents.
Comment 5 Fabio D'Urso 2013-05-14 07:46:11 UTC
Git commit 2ae9e58bb4fd339feef5ec805d4c5230eb8fbba5 by Fabio D'Urso, on behalf of Jon Mease.
Committed on 14/05/2013 at 09:37.
Pushed by fabiod into branch 'master'.

Merge window.text, contents and inplaceText annotation properties
REVIEW: 110391

M  +3    -53   core/annotations.cpp
M  +0    -20   core/annotations.h
M  +1    -9    core/document.cpp
M  +1    -1    generators/djvu/generator_djvu.cpp
M  +0    -2    generators/poppler/annots.cpp
M  +1    -1    ui/pagepainter.cpp
M  +2    -3    ui/pageviewannotator.cpp

http://commits.kde.org/okular/2ae9e58bb4fd339feef5ec805d4c5230eb8fbba5