D11609: Add support for chained / next actions

Albert Astals Cid noreply at phabricator.kde.org
Tue Apr 17 07:37:25 UTC 2018


aacid added inline comments.

INLINE COMMENTS

> aheinecke wrote in generator_pdf.cpp:475
> @aacid When you commited the corresponding poppler patch you did not add this API.
> 
> The problem here is:
> 
> - In the LinkPrivate dtor we delete the nextLinks list.
> - createLinkFromPopplerLink deletes the parsed the link.
> 
> My solution for this was to add API to change the nextLinks so that they can be "taken" from a Link.
> 
> Maybe in poppler we could add "Poppler::Link::takeNextLinks()" to have a more explicit API for that?
> 
> Alternatively changing "createLinkFromPopplerLink" to optionally not delete the link it parses? I think that is a bit more error prone as we have to make sure we catch everything. For example the movie and rendition actions have their own deletion.

We have some takeXX functions but i personally don't like them much because they "break" the object, it may be very well possible that at some point we need to do getNextLinks somewhere else and we wouldn't see "we're doing it wrong" easily, since it would just be returning an empty list.

I think adding a parameter to createLinkFromPoppler is the way to go, and yes it already is not great that createLinkFromPoppler behaves "randomly" in whether it should delete or not the links it gets.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D11609

To: aheinecke, #okular
Cc: aacid, michaelweghorn, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180417/85562054/attachment-0001.html>


More information about the Okular-devel mailing list