[Poppler-bugs] [Bug 89136] Annotations of /Subtype /Popup are not added to /Annots array of a page

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=89136

--- Comment #14 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 113520
  --> https://bugs.freedesktop.org/attachment.cgi?id=113520
Add annotation of Subtype Popup to pdf page

Review of attachment 113520:
-----------------------------------------------------------------

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 @@
> +  // 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());

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 @@
>    annot->setPage(num, gTrue);
> +
> +  AnnotMarkup *annot_markup = dynamic_cast<AnnotMarkup*>(annot);

We don't use underscores for name variables in the core.

@@ +450,5 @@
> +  AnnotMarkup *annot_markup = dynamic_cast<AnnotMarkup*>(annot);
> +  if (annot_markup) {
> +    AnnotPopup *annot_popup = annot_markup->getPopup();
> +    if (annot_popup) {
> +      this->addAnnot(annot_popup);

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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20150418/377795ad/attachment.html>