Bug 55378 - [PATCH] make LinkRendition properties available in Qt4 frontend
Summary: [PATCH] make LinkRendition properties available in Qt4 frontend
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: qt4 frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-27 08:30 UTC by Tobias Koenig
Modified: 2012-10-08 20:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch to extend Poppler::LinkRendition in Qt4 frontend (6.68 KB, text/plain)
2012-09-27 08:30 UTC, Tobias Koenig
Details
Updated patch (8.42 KB, patch)
2012-09-27 12:53 UTC, Tobias Koenig
Details | Splinter Review
Updated patch (10.77 KB, patch)
2012-10-08 06:43 UTC, Tobias Koenig
Details | Splinter Review

Description Tobias Koenig 2012-09-27 08:30:36 UTC
Created attachment 67756 [details]
Patch to extend Poppler::LinkRendition in Qt4 frontend

The attached patch makes the operation and JavaScript properties of the LinkRendition class available in the Poppler::LinkRendition class of the Qt4 frontend.
Comment 1 Tobias Koenig 2012-09-27 12:53:53 UTC
Created attachment 67766 [details] [review]
Updated patch

Extended previous patch by adding test function for referenced ScreenAnnotation.
Comment 2 Albert Astals Cid 2012-10-01 20:34:05 UTC
Could you try to avoid whitespace only changes? I know you are actually fixing stuff but it makes the patches a bit harder to review.

The "delete linkAction;" removal seems like it does not belong to this patch?

We don't have an enum for the operations thing in the poppler core? Maybe it's worth adding?

Could you keep the old LinkRendition constructor and mark it as deprecated so we don't break the ABI unccessesarly?
Comment 3 Tobias Koenig 2012-10-08 06:43:22 UTC
Created attachment 68242 [details] [review]
Updated patch

I have updated the patch according to the review comments now. The 'delete link' is still in place, because deleting the link object at this place is wrong and just worked by coincidence. Using the new LinkRendition object with a ScreenAnnotation uncovers that.
The constructor of LinkRendition still takes an integer for the operation parameter, because I don't see a clean way to make the enum from the ::LinkRendition class available in the header file. Internally however the new enum is used.
Comment 4 Albert Astals Cid 2012-10-08 20:33:26 UTC
Commited :-)


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.