D11723: Add FormFieldSignature to Okular namespace

[Albert Astals Cid]

aacid added a comment.


  I understand you're mimicing the poppler API at some points but that doesn't always makes sense since poppler is a multi-purpose library and okular is a document viewer.

INLINE COMMENTS

> form.cpp:284
> +
> +        void setValue( const QString& v ) override
> +        {

What's this used for?

> form.h:390
> +            ValidateVerifyCertificate = 1, ///< Validate the certificate.
> +            ValidateForceRevalidation = 2, ///< Force revalidation of the certificate.
> +        };

What would be the usecase for this force?

> form.h:405
> +         */
> +        virtual QMap<QString, QVariant> validate( ValidateOptions opt ) const = 0;
> +

a map of strings to variants is not very good API since basically it can have any random things in there. So as a consumer of that API you're blind. Wouldn't a class/structure make sense here?

> form.h:412
> +         */
> +        virtual QMap<QString, QVariant> validate( int opt, const QString& validationTime ) const = 0;
> +

As a viewer when would we need to have a different validation time than now?

> formfields.cpp:389
> +        case PopplerSignatureValidationInfo::SignatureValid:
> +            signStat =  QStringLiteral( "Signature is Valid." );
> +            break;

Are these strings supposed to be user visible? If so you need i18n around them

> formfields.h:140
> +
> +        enum SignatureStatus {
> +            SignatureValid,          ///< The signature is cryptographically valid.

Why do we need all these enums can't we use the ones in poppler¿

REPOSITORY
  R223 Okular

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

To: chinmoyr, aacid
Cc: ngraham, okular-devel, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180518/bc90c416/attachment.html>