D21202: Added basic JavaScript functions to support animated PDF

Joao Oliveira noreply at phabricator.kde.org
Thu May 16 12:42:31 BST 2019


joaonetto added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kjs_display.cpp:19
> use unique_ptr

Are we talking about the std::unique_ptr?

I followed the template in the other classes, why should we use unique_ptr?

> anthonyfieroni wrote in kjs_display.cpp:35-38
>   if (g_displayProto)
>       return;

Then we get rid of the initialized bool?

> anthonyfieroni wrote in kjs_display.cpp:40
>   g_displayProto.reset(new KJSPrototype);

And this does not compile

> pino wrote in kjs_document.cpp:275-286
> This convoluted for loop basically get the i-th element of a QLinkedList, which is not designed for index accessing (and that is why there is no at(int) method).
> IMHO a better way is:
> 
> - get the list of form fields
> - compare the wanted index with the count of the form fields: if it is out of boundaries, return KJSUndefined directly
> - otherwise, create an iterator fromthe begin, and increment it by the wanted index to get to the wanted element

I still have to check for all the pages.

But my new implementation looks more like with what you described

REPOSITORY
  R223 Okular

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

To: joaonetto, aacid, #okular
Cc: pino, anthonyfieroni, sander, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190516/c033f77c/attachment.html>


More information about the Okular-devel mailing list