Created attachment 138372 [details] [review] Patch to add support for the hide action This adds the action "Hide" as specified in Adobe PFD Reference Version 1.7 Section 8.5.3 (page 665f.) Hide Actions can be used to show or hide form fields. It is part of a changeset for Okular to support such actions. See https://phabricator.kde.org/T8274 for details. This is why the qt5 binding is also extended as I used this for testing. The task also contains a test document. The Okular task contains an autotest for this feature. ---- A Problem I had when implementing this patch was that according to Adobe's reference the "T" value can also be "An indirect reference to an annotation dictionary" or "An array of such dictionaries". I was not 100% sure what this meant for parsing or how the API should look like for that. With Adobe Acrobat DC I was not able to create an example containing such a thing. So I've only implemented API for a Named target and commented in the Action's declaration. I think this is the best in such a situation as in the future the API can be extended, if necessary, without breaking something.
Thanks and sorry for the delay on reviewing! I was wondering if to future proof the Qt5 API and its users (the core API can be changed as much as we want), it would make sense to change QString targetName() const; be QVector<QString> targets() const; And for now we just return one value. This way when we implement cases b/c the application that uses poppler-qt5 will hopefully magically just work? Also can you please change the Qt5::LinkHide constructor to be like the LinkOCGState one, i.e. so that it gets the private passed in, this way it's clear "normal users" should not create one, and if we need to pass more stuff to the constructor we don't break ABI or anything since it's still a pointer.
Created attachment 138702 [details] [review] Patch to add support for the hide action - Updated 1 (In reply to Albert Astals Cid from comment #1) > Thanks and sorry for the delay on reviewing! No problem. Thanks for the review :-) > I was wondering if to future proof the Qt5 API and its users (the core API > can be changed as much as we want), it would make sense to change > QString targetName() const; > be > QVector<QString> targets() const; > > And for now we just return one value. This way when we implement cases b/c > the application that uses poppler-qt5 will hopefully magically just work? Good idea. I'll change the okular patch accordingly. > Also can you please change the Qt5::LinkHide constructor to be like the > LinkOCGState one, i.e. so that it gets the private passed in, this way it's > clear "normal users" should not create one, and if we need to pass more > stuff to the constructor we don't break ABI or anything since it's still a > pointer. Changed. I didn't understand why the OCGState used this pattern and oriented myself on the others ;-)
Pushed (with some improvements)
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.