I think some extra comment lines explaining why this code is necessary would be helpful here.

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 effects have to be mitigated also locally to PDFGenerator. PDFGenerator::swapBackingFile is the most obvious place.

@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) for reload. Hot-swapping replaces in-memory core document data, while leaving the UI unaffected (stay at current page, don't reload rasterized page images).

Each Page object has a list of rectangular areas (Page::m_rects), that indicate where the UI must be sensitive for mouse-over and click. The rectangles refer to different objects, like annotations and actions. In the case of PDF links, a generator is supposed to setup these rectangular areas as ObjectRect::Action and push them into a Page via Okular::Page::setObjectRects.

PDFGenerator deferrers link setup. Initially on document load, links are cleared and empty. Link setup is postponed until a page is first rendered with PDFGenerator::image. Page rendering happens on-demand (depending on PageView scroll position, thumbnails preview, ...) and is async and unrelated to document loading. The first render request for a page causes all links for that page to be setup. The PDFGenerator has a bit array rectsGenerated to remember which page has already been setup with links (one bit per page).

In the bug, Page::m_rects was empty after hot-swapping, because PDFGenerator depends on PDFGenerator::image to generate them. But PDFGenerator::image did not happen, as it is deliberately not tied to hot-swap. So the links are potentially never generated.

Some more details on hot-swapping: Document::swapBackingFile calls Generator::swapBackingFile to generate new Page models. It then moves the d-Pointer and most inner content of the new (temporary) pages into the previously existing Page objects. The existing Page objects are kept intact, because they're referenced by UI. Most old data is deleted, but few things like Page::m_pixmaps and Page::m_boundingBox are preserved from the former Pages. Most interestingly, link rectangles in Page::m_rects are replaced with the new m_rects from Generator.

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.

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 anybody have such files laying around?

Once that's done, am I supposed to do something else, like accepting the revision? Or even committing it? Sorry, was not involved in that late Diff stages yet...

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.