[poppler] [PATCH] Add setContents() to modify the annot contents

[Carlos Garcia Campos]

El mié, 22-04-2009 a las 00:36 +0200, Albert Astals Cid escribió:
> A Dimarts, 21 d'abril de 2009, Carlos Garcia Campos va escriure:
> > El lun, 20-04-2009 a las 21:56 +0200, Albert Astals Cid escribió:
> > > A Dilluns, 20 d'abril de 2009, Carlos Garcia Campos va escriure:
> > > > El dom, 19-04-2009 a las 23:41 +0200, Albert Astals Cid escribió:
> > > > > A Dissabte, 18 d'abril de 2009, Carlos Garcia Campos va escriure:
> > > > > > Hi all,
> > > > > >
> > > > > > we are still lacking write support in annotations. This patch, to
> > > > > > be able to modify the contents of an annotation, it's just a first
> > > > > > step.
> > > > > >
> > > > > > ok to commit?
> > > > >
> > > > > contents = new GooString(new_content);
> > > > >
> > > > > crashes if new_content is null, so you either protect this or remove
> > > > > the protection in
> > > >
> > > > fair enough. It makes me wonder what should we do in this case, I mean,
> > > > the Contents field is optional in the annotation array, should we
> > > > remove the key from the dict when NULL is set? or shouldn't we even
> > > > allow passing NULL to setContents()?
> > >
> > > I just would mark on the header that passing a NULL value to set contents
> > > is not ok, if you want to clear something just pass GooString() and
> > > that's all
> > >
> > > > > if (new_content && !contents->hasUnicodeMarker()) {
> > > > >
> > > > > I think obj1 is missing a free().
> > > >
> > > > No, because it's owned by the Dict.
> > >
> > > Right, i got tricked thinking that operator= in Object was the same as
> > > copy.
> > >
> > > > > Also you should probably update the modified member too.
> > > >
> > > > What do you mean with the modified member?
> > >
> > > GooString *modified; inside class Annot.
> >
> > I have committed the path without this for the moment. In page 1113 of
> > the pdf spec it says: "Acrobat viewers update the annotation
> > dictionary’s M entry only for text
> > annotations"
> >
> > Should we follow this and implement it only for text annotations?
> 
> Hmpf, any reason in favor or against that? I'm leaning to doing it for all of 
> them, unless someone knows why Acrobat decided to do it only for text and 
> seems to make sense.

I also think it makes more sense to always update M when the annot is
modified. 

> BTW could you add a comment into the header near setContents saying the 
> ownership of the GooString is not taken and so the caller is responsible of 
> deleting it?

Sure, I forgot it.

Attached is a new set of patches, ok to commit?

> Albert
> 
> >
> > > Albert
> > >
> > > > > Otherwise it think this can't hurt to let it go in :D
> > > >
> > > > Great, I'll commit it when I know what to do with the NULL case.
> > > >
> > > > Thanks.
> > > >
> > > > > Albert
> > > > > _______________________________________________
> > > > > poppler mailing list
> > > > > poppler at lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/poppler
> > >
> > > _______________________________________________
> > > poppler mailing list
> > > poppler at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/poppler
> 
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
-- 
Carlos Garcia Campos
   elkalmail at yahoo.es
   carlosgc at gnome.org
   http://carlosgc.linups.org
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
-------------- next part --------------
A non-text attachment was scrubbed...
Name: poppler-annot-set-contents.diff
Type: text/x-patch
Size: 6329 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090423/f08f43f4/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada
 digitalmente
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090423/f08f43f4/attachment.pgp