D14752: Fix links being "lost" on save

This revision was automatically updated to reflect the committed changes. Closed by commit R223:0a8d2f7f8575: Fix links being "lost" on save (authored by aacid). REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14752?vs=39574&id=40488 REVISION DETAIL https://p

D14752: Fix links being "lost" on save

tobiasdeiminger accepted this revision. tobiasdeiminger added a comment. This revision is now accepted and ready to land. Here's a PDF and LaTex source , suitable to trigger and test OkularLinkType::s

D14752: Fix links being "lost" on save

aacid added a comment. If you like the code, you just approve it. And if it's someone that doesn't have commit rights you usually push it too. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D14752 To: aacid Cc: tobiasdeiminger, sander, okular-devel, ngraham, aaci

D14752: Fix links being "lost" on save

tobiasdeiminger added a comment. I'm currently looking for documents to test the resolveMediaLinkReferences stuff. It requires documents containing pages, screen/widget annotations, or form fields - with associated open or close additional action, of action type movie or rendition. Does anyb

D14752: Fix links being "lost" on save

aacid added inline comments. INLINE COMMENTS > tobiasdeiminger wrote in generator_pdf.cpp:653 > Guess this is the same sanity check as in document.cpp, > Document::swapBackingFile, probably meaning "is it still the same document?". > Could there be situations where old page count != new page co

D14752: Fix links being "lost" on save

aacid updated this revision to Diff 39574. aacid added a comment. add small comment Fix leak of poppler page REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14752?vs=39477&id=39574 BRANCH arcpatch-D14752 REVISION DETAIL https://phabricator.kde.org/

D14752: Fix links being "lost" on save

sander added a comment. That's more than enough as an explanation, and too much to put it in the text. Can we simply mention the bug and diff numbers in the code? IMO git blame is really only useful until somebody applies another change to the lines this patch introduces. REPOSITORY

D14752: Fix links being "lost" on save

tobiasdeiminger added inline comments. INLINE COMMENTS > generator_pdf.cpp:653 > > +if (oldRectsGenerated.count() == rectsGenerated.count()) { > +for (int i = 0; i < oldRectsGenerated.count(); ++i) { Guess this is the same sanity check as in document.cpp, Document::swapBackingFile

D14752: Fix links being "lost" on save

tobiasdeiminger added a comment. @sander Here's a bit of explanation how I understand it. Maybe you can extract comments / commit message / documentation from it. "ctrl+s" saves a modified PDF to file, and then uses "hot-swap" (43a3756e1

D14752: Fix links being "lost" on save

tobiasdeiminger added a comment. Alberts patch works for me and I don't know another solution that would be reasonable simple. BUG: 397373 is a side effect of deferred ("lazy") link setup inside PDFGenerator. Deferred link setup is an implementation detail of PDFGenerator, and so its side ef

D14752: Fix links being "lost" on save

sander added a comment. I think some extra comment lines explaining why this code is necessary would be helpful here. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D14752 To: aacid Cc: sander, okular-devel, ngraham, aacid

D14752: Fix links being "lost" on save

aacid created this revision. Restricted Application added a project: Okular. Restricted Application added a subscriber: okular-devel. aacid requested review of this revision. REVISION SUMMARY We need to regenerate the links when switching the file since we won't re-render the pages since we al