<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Extend support for free text annotations." href="https://bugs.freedesktop.org/show_bug.cgi?id=81665#c5">Comment # 5</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Extend support for free text annotations." href="https://bugs.freedesktop.org/show_bug.cgi?id=81665">bug 81665</a> from <a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> Carlos Garcia Campos</a> <pre>Comment on <a href="attachment.cgi?id=103327" name="attach_103327" title="glib: Extend FreeText annotation support">attachment 103327</a> <a href="attachment.cgi?id=103327&action=edit" title="glib: Extend FreeText annotation support">[details]</a> <a href='page.cgi?id=splinter.html&bug=81665&attachment=103327'>[review]</a> glib: Extend FreeText annotation support Review of <a href="attachment.cgi?id=103327" name="attach_103327" title="glib: Extend FreeText annotation support">attachment 103327</a> <a href="attachment.cgi?id=103327&action=edit" title="glib: Extend FreeText annotation support">[details]</a> <a href='page.cgi?id=splinter.html&bug=81665&attachment=103327'>[review]</a>: ----------------------------------------------------------------- ::: glib/poppler-annot.cc @@ +472,5 @@ > > +static GooString *create_appearance_string (PopplerAnnotAppearance *appearance) > +{ > + if (!appearance) > + return new GooString ("/Invalid_font 12 Tf"); Doesn't this fail? It's using an invalid font @@ +495,5 @@ > + * poppler_page_add_annot() > + * > + * Return value: A newly created #PopplerAnnotFreeText annotation > + * > + * Since: 0.27 Use 0.28 instead @@ +499,5 @@ > + * Since: 0.27 > + */ > +PopplerAnnot * > +poppler_annot_free_text_new (PopplerDocument *doc, > + PopplerRectangle *rect) The DA entry is a required field, so I think we should pass the font name, color and size as parameters of the constructor @@ +1703,5 @@ > + * poppler_annot_free_text_set_quadding: > + * @poppler_annot: a #PopplerAnnotFreeText > + * @quadding: a #PopplerAnnotFreeTextQuadding > + * > + * Retrieves the intent of the @poppler_annot. Copy paste issue here :-) @@ +1705,5 @@ > + * @quadding: a #PopplerAnnotFreeTextQuadding > + * > + * Retrieves the intent of the @poppler_annot. > + * > + * Return value: #PopplerAnnotFreeTextIntent of @poppler_annot. Ditto, this is a void function @@ +1723,5 @@ > + > +/** > + * poppler_annot_free_text_get_inent: > + * @poppler_annot: a #PopplerAnnotFreeText > + * @quadding: a #PopplerAnnotFreeTextIntent This parameter doesn't exist @@ +1742,5 @@ > + > +/** > + * poppler_annot_free_text_set_intent: > + * @poppler_annot: a #PopplerAnnotFreeText > + * @quadding: a #PopplerAnnotFreeTextIntent This should be @intent @@ +1806,5 @@ > > +/** > + * poppler_annot_free_text_get_callout_line: > + * @poppler_annot: a #PopplerAnnotFreeText > + * @callout: a #PopplerAnnotCalloutLine Since this can be NULL to unset the callout line, add (allow none) tag and document that this can be NULL and the effects of passing NULL. @@ +1834,5 @@ > + > + annot = static_cast<AnnotFreeText *>(POPPLER_ANNOT (poppler_annot)->annot); > + annot->setCalloutLine (line); > + if (!line) > + delete line; if (line) @@ +1862,5 @@ > + annot = static_cast<AnnotFreeText *>(POPPLER_ANNOT (poppler_annot)->annot); > + da = annot->getAppearanceString(); > + > + if (!da || da->getLength() == 0) > + g_error ("The free text annotation has no DA"); This can't happen, the core already fails to create the annot in this case (or it should), so we can safely assume here there's a da. Otherwise, use an assert instead of g_error</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>