Add FormFieldSignature to Okular namespace
AbandonedPublic

Authored by chinmoyr on Mar 26 2018, 5:44 PM.

Details

Reviewers
aacid
Summary

This patch adds the abstract class FormFieldSignature, implements it in poppler generator and modifies poppler generator to recognize signature
form field.

Diff Detail

Repository
R223 Okular
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
chinmoyr created this revision.Mar 26 2018, 5:44 PM
Restricted Application added a project: Okular. · View Herald TranscriptMar 26 2018, 5:44 PM
chinmoyr requested review of this revision.Mar 26 2018, 5:44 PM
chinmoyr retitled this revision from [RFC] Add FormFieldSignature to Okular's namespace to Add FormFieldSignature to Okular namespace.May 14 2018, 4:16 PM
chinmoyr edited reviewers, added: aacid; removed: Okular.
Restricted Application added a subscriber: okular-devel. · View Herald TranscriptMay 14 2018, 4:16 PM
chinmoyr updated this revision to Diff 34307.May 16 2018, 5:28 PM

Removed code unrelated to this revision.

aacid added a comment.May 18 2018, 1:28 PM

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.

core/form.cpp
307

What's this used for?

core/form.h
415

What would be the usecase for this force?

430

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?

437

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

generators/poppler/formfields.cpp
458

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

generators/poppler/formfields.h
146

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

chinmoyr updated this revision to Diff 34717.May 23 2018, 2:55 PM
chinmoyr marked 2 inline comments as done.

Updated API.
Removed cases with no use cases.
Removed redundant enums.

chinmoyr added inline comments.May 23 2018, 2:57 PM
core/form.cpp
307

For other formfields it can be used to set their default states. But for signature field I have no idea.

core/form.h
415

I never thought of a possible use case before duplicating it here. For now I have removed it from the patch. I will update this diff if I find any use case.

437

Same here. Never actually thought of a use case and still can't find one.

aacid added inline comments.May 23 2018, 10:54 PM
core/signatureinfo.h
34

int is bad, this, certificateStatus and hashAlgorithm need to be enums, what am i supposed to do with this returning 3? What does 3 mean?

59

time_t is bad, we want QDateTime

chinmoyr updated this revision to Diff 35040.May 28 2018, 5:29 PM

Reused enums of Poppler::SignatureValidationInfo.
Used QDateTime in place of time_t.

aacid added inline comments.May 31 2018, 1:21 PM
generators/poppler/formfields.cpp
433

Why a static_cast here?

generators/poppler/pdfsignatureinfo.cpp
44

You need to delete d_ptr?

sander added a subscriber: sander.May 31 2018, 1:23 PM
sander added inline comments.
generators/poppler/pdfsignatureinfo.cpp
44

Or use a std::unique_ptr?

chinmoyr updated this revision to Diff 35510.Jun 4 2018, 8:57 AM

Removed the unnecessary static_cast
Used QScopedPointer for d_ptr

chinmoyr updated this revision to Diff 35513.Jun 4 2018, 11:10 AM

Added validateSignatures() to validate signatures while loading a page.
Added to document info whether or not a pdf is digitally signed.

chinmoyr updated this revision to Diff 35983.Jun 11 2018, 1:15 AM

Revert to previous version. Now signatures will not be validated upfront.

chinmoyr abandoned this revision.Jun 26 2018, 5:00 PM

Outdated revision