<table><tr><td style="">aacid added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D11609">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11609#inline-62051">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aheinecke</span> wrote in <span style="color: #4b4d51; font-weight: bold;">generator_pdf.cpp:475</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;"><a href="https://phabricator.kde.org/p/aacid/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@aacid</a> When you commited the corresponding poppler patch you did not add this API.</p>

<p style="padding: 0; margin: 8px;">The problem here is:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">In the LinkPrivate dtor we delete the nextLinks list.</li>
<li class="remarkup-list-item">createLinkFromPopplerLink deletes the parsed the link.</li>
</ul>

<p style="padding: 0; margin: 8px;">My solution for this was to add API to change the nextLinks so that they can be "taken" from a Link.</p>

<p style="padding: 0; margin: 8px;">Maybe in poppler we could add "Poppler::Link::takeNextLinks()" to have a more explicit API for that?</p>

<p style="padding: 0; margin: 8px;">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.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p>

<p style="padding: 0; margin: 8px;">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.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11609">https://phabricator.kde.org/D11609</a></div></div><br /><div><strong>To: </strong>aheinecke, Okular<br /><strong>Cc: </strong>aacid, michaelweghorn, ngraham<br /></div>