<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Annotations of /Subtype /Popup are not added to /Annots array of a page"
href="https://bugs.freedesktop.org/show_bug.cgi?id=89136#c14">Comment # 14</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Annotations of /Subtype /Popup are not added to /Annots array of a page"
href="https://bugs.freedesktop.org/show_bug.cgi?id=89136">bug 89136</a>
from <span class="vcard"><a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=113520" name="attach_113520" title="Add annotation of Subtype Popup to pdf page">attachment 113520</a> <a href="attachment.cgi?id=113520&action=edit" title="Add annotation of Subtype Popup to pdf page">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=89136&attachment=113520'>[review]</a>
Add annotation of Subtype Popup to pdf page
Review of <span class=""><a href="attachment.cgi?id=113520" name="attach_113520" title="Add annotation of Subtype Popup to pdf page">attachment 113520</a> <a href="attachment.cgi?id=113520&action=edit" title="Add annotation of Subtype Popup to pdf page">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=89136&attachment=113520'>[review]</a>:
-----------------------------------------------------------------
Sorry for the delay reviewing this, I've been quite busy. I've addressed my
comments and pushed to git master. Let's fix the removal in a follow up patch
if needed.
::: poppler/Annot.cc
@@ +2093,5 @@
<span class="quote">> + // associated with a page, then we need to remove that
> + // popup annotation from the page. Otherwise we would have
> + // dangling references to it.
> + if (popup != NULL && popup->getPageNum() != 0) {
> + Page *pageobj = popup->getDoc()->getPage(popup->getPageNum());</span >
I guess we can use doc here instead of getting the doc from the popup, it must
be the same doc.
::: poppler/Page.cc
@@ +448,3 @@
<span class="quote">> annot->setPage(num, gTrue);
> +
> + AnnotMarkup *annot_markup = dynamic_cast<AnnotMarkup*>(annot);</span >
We don't use underscores for name variables in the core.
@@ +450,5 @@
<span class="quote">> + AnnotMarkup *annot_markup = dynamic_cast<AnnotMarkup*>(annot);
> + if (annot_markup) {
> + AnnotPopup *annot_popup = annot_markup->getPopup();
> + if (annot_popup) {
> + this->addAnnot(annot_popup);</span >
So, this makes me wonder, should we also remove the popup associated to a
markup annotation in Page::removeAnnot() when a markup annot is given?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>