[Poppler-bugs] [Bug 28780] Add getLabel method to FormWidget

[bugzilla-daemon at freedesktop.org]

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

--- Comment #6 from Carlos Garcia Campos <carlosgc at gnome.org> 2010-08-03 02:47:39 PDT ---
(From update of attachment 37487)
Some comments below

>diff --git a/glib/poppler-form-field.cc b/glib/poppler-form-field.cc
>index 33c4b15..e774880 100644
>--- a/glib/poppler-form-field.cc
>+++ b/glib/poppler-form-field.cc
>@@ -220,6 +220,67 @@ poppler_form_field_button_set_state (PopplerFormField *field,
>   static_cast<FormWidgetButton*>(field->widget)->setState ((GBool)state);
> }
> 
>+/**
>+ * poppler_form_field_get_fully_qualified_name:
>+ * @field: a #PopplerFormField
>+ *
>+ * Gets the fully qualified name (concatenated PDF /T properties) for @field.
>+ *
>+ * Return value: a new allocated string. It must be freed with g_free() when done.
>+ **/
>+gchar*
>+poppler_form_field_get_fully_qualified_name (PopplerFormField *field)
>+{
>+  GooString *tmp;
>+
>+  g_return_val_if_fail (POPPLER_IS_FORM_FIELD (field), NULL);
>+  
>+  tmp = field->widget->getFullyQualifiedName();
>+
>+  return tmp ? _poppler_goo_string_to_utf8 (tmp) : NULL;
>+}

I think we can just call this poppler_form_field_get_name()


>+GooString* FormWidget::getFullyQualifiedName() {
>+  Object obj1, obj2;
>+  Object parent;
>+  GooString *parent_name;
>+  GooString *full_name;
>+
>+  if (fullyQualifiedName)
>+    return fullyQualifiedName;
>+
>+  if (!partialName)
>+    return NULL;

this is not correct, according to the spec: 

"It is possible for different field dictionaries to have the same fully
qualified field name if they are descendants of
a common ancestor with that name and have no partial field names (T entries) of
their own."

so, even if the field doesn't have a partial name, it might have a fully
qualified name if its parent has a partial name. 

>+  full_name = new GooString(partialName);

why not building the full name first and then append the partial name if there
is one?

>+  obj.copy(&obj1);
>+  while (obj1.getDict()->lookup("Parent", &parent)->isDict()) {
>+    if (parent.getDict()->lookup("T", &obj2)->isString()) {

Use Object::dictLookup() rather than getDict()->lookup() in both cases here.

>+      parent_name = obj2.getString()->copy();
>+      obj2.free();
>+
>+      if (full_name->hasUnicodeMarker()) {
>+        full_name->del(0, 2); // Remove the unicode BOM
>+        parent_name->append("\0.", 2); // 2-byte unicode period
>+      } else {
>+        parent_name->append('.'); // 1-byte ascii period
>+      }
>+
>+      parent_name->append(full_name);
>+
>+      delete full_name;
>+      full_name = parent_name;

We can avoid duplicating the string and deleting all the time, by using
GooString::insert() instead of append. We insert the period first, and then the
partial name directly into the full_name GooString.

>+    }
>+    obj1.free();
>+    parent.copy(&obj1);
>+    parent.free();
>+  }
>+  fullyQualifiedName = full_name;

we should make sure the fullyQualifiedName is not an empty string because in
that case we should delete the string and return NULL.

>+  return fullyQualifiedName;
>+}
>+

Thank you very much for the patch.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.