[poppler] Form-Reset and Print [patches]

[Albert Astals Cid]

A Dilluns, 14 de juny de 2010, Guillermo Amaral va escriure:
> On Sun, Jun 13, 2010 at 11:02:56PM +0100, Albert Astals Cid wrote:
> > A Diumenge, 13 de juny de 2010, Guillermo Amaral va escriure:
> > > On Sun, Jun 13, 2010 at 10:21:35AM +0200, Carlos Garcia Campos wrote:
> > > > Excerpts from Guillermo Amaral's message of dom jun 13 04:40:44 +0200
> > 
> > 2010:
> > > > > Albert Astals Cid <aacid at kde.org> wrote:
> > > > > > A Dissabte, 12 de juny de 2010, Guillermo Amaral va escriure:
> > > > > >> I wanted to send in these patches, they allow handling of
> > > > > >> reset-form and print actions respectably.
> > > > > >> 
> > > > > >> Please check them out, and if you have any questions please
> > > > > >> don't hesitate to ask.
> > > > > > 
> > > > > > Do you have a document that uses them?
> > > > > > 
> > > > >   I do, but I dunno if I can share them ATM, looking into it.
> > > > >   But I did make a test pdf that uses both features. :-)
> > > > >   
> > > > > > Also what is the point of
> > > > > > 
> > > > > > +  } else if (obj2.isName("ResetForm")) {
> > > > > > +    action = new LinkResetForm(obj2.getName());
> > > > > > 
> > > > >   As usual, you are right aacid :) I try a little to hard not to
> > > > >   break the flow of other programs, I think I went to far lol
> > > > >   (fixed)
> > > > >   
> > > > > > And
> > > > > > if (obj->dictLookup("AA", &tmp)->isDict())
> > > > > > seems like an unrelated fix, can you please explain it too?
> > > > > > 
> > > > >   Mixed them up, that one should be in the print patch (moved).
> > > > >   Sometimes the AA dict is used to hold a ref to the named object.
> > > > >   I have seen it in two docs so far, it seems to happen when
> > > > >   people set an action to happen on mouse down instead of mouse
> > > > >   up. It should not break anything else (as far as I can see).
> > > > 
> > > > We should definitely add support for additional actions properly in
> > > > poppler.
> > > > 
> > > > > diff --git a/poppler/Link.cc b/poppler/Link.cc
> > > > > index 5d7b779..e3d6ce7 100644
> > > > > --- a/poppler/Link.cc
> > > > > +++ b/poppler/Link.cc
> > > > > @@ -124,6 +124,10 @@ LinkAction *LinkAction::parseAction(Object
> > > > > *obj, GooString *baseURI) {
> > > > > 
> > > > >    } else if (obj2.isName("SetOCGState")) {
> > > > >    
> > > > >      action = new LinkOCGState(obj);
> > > > > 
> > > > > +  // ResetForm action
> > > > > +  } else if (obj2.isName("ResetForm")) {
> > > > > +    action = new LinkResetForm();
> > > > > +
> > > > > 
> > > > >    // unknown action
> > > > >    } else if (obj2.isName()) {
> > > > >    
> > > > >      action = new LinkUnknown(obj2.getName());
> > > > > 
> > > > > @@ -856,6 +860,18 @@ LinkOCGState::StateList::~StateList() {
> > > > > 
> > > > >  }
> > > > >  
> > > > >  //----------------------------------------------------------------
> > > > >  ---- ----
> > > > > 
> > > > > +// LinkFormClear
> > > > > +//----------------------------------------------------------------
> > > > > ---- ---- +
> > > > > +LinkResetForm::LinkResetForm() {
> > > > > +  action = new GooString("ResetForm");
> > > > 
> > > > this is redundant, you already know it's a LinkResetForm object. You
> > > > should parse Fields and Flags here.
> > > > 
> > >   Yeah it's a partial patch, it allows the client to know it has to
> > >   reset.. I guess I could make it reset the fields in the fieldlist
> > >   here and just notify the client the form values have changed or
> > >   something? what do you guys think?
> > >   
> > >   As far as it being redundant T_T yep totally. but I do see that
> > >   the getAction() accessor returns said action GooString ptr, as I said
> > > 
> > > before I don't want to rock the boat; I'm guessing that method is used
> > > some place else so I don't want to take it out willy nilly. But I do
> > > want to make it clear (if it's not totally obvious already) that I'm
> > > not completely familiar with the popplers code. ;-)
> > 
> > getAction() used somewhere? It's not virtual and you don't use it
> > anywhere, so where is it going to be used?
> 
>   Mmm thought it was lol, dammit.
>   Anyway, I added the missing features to the reset form patch today, check
> it out.
> 
>   Cheers,
>   GA

Some more comments:
+      fieldList->append(obj2.getString()->copy());
   you need to check obj2 is a string before doing this

+  if (obj->dictLookup("Flags", &obj1)->isNum()) {
+    flags = obj1.getInt();
+  }
Here you check for num but then getInt, you probably want to go with isInt

Albert