<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib"
href="https://bugs.freedesktop.org/show_bug.cgi?id=64821#c75">Comment # 75</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib"
href="https://bugs.freedesktop.org/show_bug.cgi?id=64821">bug 64821</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=94991" name="attach_94991" title="[PATCH v18 3/5] glib: Accessors for form fields of structure elements">attachment 94991</a> <a href="attachment.cgi?id=94991&action=edit" title="[PATCH v18 3/5] glib: Accessors for form fields of structure elements">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=64821&attachment=94991'>[review]</a>
[PATCH v18 3/5] glib: Accessors for form fields of structure elements
Review of <span class=""><a href="attachment.cgi?id=94991" name="attach_94991" title="[PATCH v18 3/5] glib: Accessors for form fields of structure elements">attachment 94991</a> <a href="attachment.cgi?id=94991&action=edit" title="[PATCH v18 3/5] glib: Accessors for form fields of structure elements">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=64821&attachment=94991'>[review]</a>:
-----------------------------------------------------------------
This patch doesn't address or replies to my comments of the previous review.
::: glib/poppler-page.cc
@@ +1385,5 @@
<span class="quote">> + * @structure_element: a #PopplerStructureElement.
> + *
> + * Obtains the #PopplerFormFieldMapping for a single form field associated
> + * with a structure element of type %POPPLER_STRUCTURE_ELEMENT_FORM. The
> + * returned value must be freed with g_object_unref().</span >
PopplerFormFieldMapping is not a GObject, but a boxed type, that should be
freed with poppler_form_field_mapping_free().
::: glib/poppler-page.h
@@ +89,4 @@
<span class="quote">> cairo_surface_t *poppler_page_get_image (PopplerPage *page,
> gint image_id);
> GList *poppler_page_get_form_field_mapping (PopplerPage *page);
> +PopplerFormFieldMapping*</span >
PopplerFormFieldMapping* -> PopplerFormFieldMapping *
::: glib/poppler-structure-element.cc
@@ +783,5 @@
<span class="quote">> + g_return_val_if_fail (POPPLER_IS_STRUCTURE_ELEMENT (poppler_structure_element), NULL);
> + g_return_val_if_fail (poppler_structure_element->elem != NULL, NULL);
> +
> + if (poppler_structure_element->elem->getType () != StructElement::Form)
> + return NULL;</span >
As I said in the previous review, I think this is a programmer error, it should
use a g_return macro.
@@ +804,5 @@
<span class="quote">> + }
> + else
> + {
> + // Element contains the form field ID as the MCID attribute.
> + field_id = child->getMCID ();</span >
Please, see my previous review.
::: glib/reference/poppler-sections.txt
@@ +611,4 @@
<span class="quote">> poppler_structure_element_get_alt_text
> poppler_structure_element_get_actual_text
> poppler_structure_element_get_text_spans
> +poppler_structure_element_form_get_field</span >
You should add PopplerPage new api to the docs too.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>