[Poppler-bugs] [Bug 64821] [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib

[bugzilla-daemon at freedesktop.org]

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

--- Comment #68 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 94908
  --> https://bugs.freedesktop.org/attachment.cgi?id=94908
[PATCH v17 4/5] glib: Accessors for document structure references

Review of attachment 94908:
-----------------------------------------------------------------

::: glib/poppler-structure-element.cc
@@ +931,5 @@
> + *
> + * Return value: Whether the element is a reference to another object.
> + */
> +gboolean
> +poppler_structure_element_is_reference (PopplerStructureElement *poppler_structure_element)

I would avoid this generic api, since we don't have a way to get the referenced
object when it's not link, annot or form field

@@ +947,5 @@
> + * Return value: The type of object pointed to by the reference, a value of
> + *    #PopplerStructureReference.
> + */
> +PopplerStructureReference
> +poppler_structure_element_get_reference_type (PopplerStructureElement *poppler_structure_element)

Same here, we should use the referenced object only for structure elements that
can contain children that are object references, but knowing what objects we
are looking for.

@@ +1002,5 @@
> +  return NULL;
> +}
> +
> +/**
> + * poppler_structure_element_get_reference_link:

The method name is link_action. I prefer to name this just link, though as this
is what we do already in our api, poppler_get_link_mapping actually returns a
list of LinkActions

@@ +1015,5 @@
> +{
> +  g_return_val_if_fail (POPPLER_IS_STRUCTURE_ELEMENT (poppler_structure_element), NULL);
> +  g_return_val_if_fail (poppler_structure_element->elem != NULL, NULL);
> +
> +  AnnotLink *link = _poppler_structure_element_find_annot_link (poppler_structure_element);

Maybe I'm missing something from the spec, but why do we have to find the link?
Shouldn't the link structure element reference a link annotation object?

@@ +1028,5 @@
> + *    object, or %NULL if the element is not a reference pointing to
> + *    a link.
> + */
> +PopplerLinkMapping *
> +poppler_structure_element_get_reference_link_mapping (PopplerStructureElement *poppler_structure_element)

I have the same concerns about this API as with the form field mapping

::: glib/poppler-structure-element.h
@@ +92,5 @@
> +typedef enum {
> +  POPPLER_STRUCTURE_REFERENCE_UNKNOWN,
> +  POPPLER_STRUCTURE_REFERENCE_ANNOT,
> +  POPPLER_STRUCTURE_REFERENCE_LINK,
> +} PopplerStructureReference;

Why are we handling this differently than form fields? Annot and Link are also
element types, so the same way I can get a form field for a FORM element, I
should be able to get the link or annot for a LINK or ANNOT element, without
any special api for references.

@@ +95,5 @@
> +  POPPLER_STRUCTURE_REFERENCE_LINK,
> +} PopplerStructureReference;
> +
> +
> +typedef struct _PopplerTextSpan PopplerTextSpan;

This looks unrelated to this patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20140301/203cc6eb/attachment.html>