> On June 1, 2013, 3:40 p.m., Albert Astals Cid wrote: > > Looks good for me besides one small thing, in editFormList and > > editFormCombo you are passing the "old" values while in editFormText and > > editFormButtons you are not (you get them from the forms themselves) is it > > possible not to pass the old values to editFormList and editFormCombo too? > > Jon Mease wrote: > This is the approach I originally attempted. However, I couldn't figure > out how to reliably get the current text out of a FormFieldChoice object > across different versions of Poppler. One complicating factor is that the > PopplerFormFieldChoice::editChoice and setEditChoice methods are only > meaningfully implemented for versions >0.22 of Poppler so getting this right > would require some ifdefs. > > That said, I'm open to implementation suggestions (and fine if you want > to change anything along these lines after committing). Thanks! > > Albert Astals Cid wrote: > I don't understand why you need to interact with Poppler* at all? I mean > the ui/* files don't so why do you need to? Isn't > FormFieldChoice::currentChoices what you need?
Sorry for the confusion. I looked at the code again it wasn't as complicated as it seemed late at night when I was trying things for the first time. And you're right, Poppler doesn't enter in. I believe v3 of the patch addresses your concern. Thanks - Jon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110589/#review33561 ----------------------------------------------------------- On June 3, 2013, 1:28 a.m., Jon Mease wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110589/ > ----------------------------------------------------------- > > (Updated June 3, 2013, 1:28 a.m.) > > > Review request for Okular. > > > Description > ------- > > Add undo / redo support for forms. Along with the previous annotation undo > support I believe this completes the implementation of undo/redo for all > document editing actions in Okular (Bug 177501). This review request > corresponds to the "Undo/Redo support in PDF forms" feature in the 4.11 > feature plan (http://techbase.kde.org/Schedules/KDE4/4.11_Feature_Plan) > > Potential issue: If the last form or annotation that was modified is outside > of the document viewport we are not currently moving the viewport to the form > or annotation and so it can be unclear what has been undone. I would > appreciate suggestions on whether we should add this viewport shifting and, > if so, how best to go about implementing it. > > > This addresses bug 177501. > http://bugs.kde.org/show_bug.cgi?id=177501 > > > Diffs > ----- > > core/document.h d443917 > core/document.cpp 2732441 > core/documentcommands.cpp 5fcc195 > core/documentcommands_p.h a9775a6 > ui/formwidgets.h 24108b8 > ui/formwidgets.cpp 57ecceb > ui/pageview.h 5e839f2 > ui/pageview.cpp 6e093ef > > Diff: http://git.reviewboard.kde.org/r/110589/diff/ > > > Testing > ------- > > Manual testing on a variety of PDFs including forms. I've attached three such > documents below. > > > File Attachments > ---------------- > > Mixed forms 1 > > http://git.reviewboard.kde.org/media/uploaded/files/2013/05/22/forms-scribus.pdf > > > http://git.reviewboard.kde.org/media/uploaded/files/2013/05/22/formSamples.pdf > Exclusive checkboxes > > http://git.reviewboard.kde.org/media/uploaded/files/2013/05/22/stripped-doc.pdf > > > Thanks, > > Jon Mease > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
