Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(68)

Issue 2260663002: Add initial Document::getAnnot support (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by tonikitoo
Modified:
1 year, 1 month ago
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Add initial Document::getAnnot support CL implements the first step in order to support Annotations manipulation in PDFium: Document::getAnnot. The method takes two arguments, an integer (page number) and a string (annotation name). When called, it iterates over the annotations on the given page number, searching for the one whose name matches the string in the second parameter. If found, then an Annot instance (see Annot.cpp/g added by this CL), is bound to a Javascript object and returned. With the use cases described in bug [1] as an initial test case, CL adds support to the following Annotation object properties: - hidden - name - type Idea is to keep evolving the implementation with more methods and properties in follow up CLs. [1] https://bugs.chromium.org/p/pdfium/issues/detail?id=492 BUG=pdfium:492 Committed: https://pdfium.googlesource.com/pdfium/+/618cb1f3e561b5d2a1dea9ec4653804f0da7267c

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressed thestig's and tsepez' feedback #

Total comments: 1

Patch Set 3 : fix GYP build #

Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -5 lines) Patch
M BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A fpdfsdk/javascript/Annot.h View 1 1 chunk +39 lines, -0 lines 0 comments Download
A fpdfsdk/javascript/Annot.cpp View 1 1 chunk +89 lines, -0 lines 0 comments Download
M fpdfsdk/javascript/Document.cpp View 1 2 2 chunks +46 lines, -0 lines 0 comments Download
M fpdfsdk/javascript/cjs_runtime.cpp View 2 chunks +3 lines, -1 line 0 comments Download
M pdfium.gyp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M testing/resources/javascript/document_methods.in View 3 chunks +13 lines, -1 line 0 comments Download
M testing/resources/javascript/document_methods_expected.txt View 2 chunks +4 lines, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 19 (9 generated)
tonikitoo
PTAL
1 year, 1 month ago (2016-08-18 21:10:54 UTC) #3
Tom Sepez
Congratulations on navigating the minefield that is CJS_. LGTM with nits. https://codereview.chromium.org/2260663002/diff/1/fpdfsdk/javascript/Annot.cpp File fpdfsdk/javascript/Annot.cpp (right): ...
1 year, 1 month ago (2016-08-18 21:45:06 UTC) #4
Lei Zhang
lgtm https://codereview.chromium.org/2260663002/diff/1/fpdfsdk/javascript/Document.cpp File fpdfsdk/javascript/Document.cpp (right): https://codereview.chromium.org/2260663002/diff/1/fpdfsdk/javascript/Document.cpp#newcode1052 fpdfsdk/javascript/Document.cpp:1052: if (!pBAAnnot) Maybe this block from here down ...
1 year, 1 month ago (2016-08-19 00:56:32 UTC) #6
tonikitoo
https://codereview.chromium.org/2260663002/diff/1/fpdfsdk/javascript/Annot.cpp File fpdfsdk/javascript/Annot.cpp (right): https://codereview.chromium.org/2260663002/diff/1/fpdfsdk/javascript/Annot.cpp#newcode37 fpdfsdk/javascript/Annot.cpp:37: bool bHidden = CPDF_Annot::IsAnnotationHidden(pPDFAnnot->GetAnnotDict()); On 2016/08/18 21:45:06, Tom Sepez ...
1 year, 1 month ago (2016-08-19 02:39:31 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2260663002/20001
1 year, 1 month ago (2016-08-19 02:39:42 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: linux_xfa_rel_gyp on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_xfa_rel_gyp/builds/388)
1 year, 1 month ago (2016-08-19 02:49:35 UTC) #12
Lei Zhang
https://codereview.chromium.org/2260663002/diff/20001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2260663002/diff/20001/BUILD.gn#newcode873 BUILD.gn:873: "fpdfsdk/javascript/Annot.cpp", Update the corresponding .gyp file. We'll remove the ...
1 year, 1 month ago (2016-08-19 02:51:31 UTC) #13
tonikitoo
On 2016/08/19 02:49:35, commit-bot: I haz the power wrote: > Try jobs failed on following ...
1 year, 1 month ago (2016-08-19 02:52:05 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2260663002/40001
1 year, 1 month ago (2016-08-19 02:56:14 UTC) #17
commit-bot: I haz the power
1 year, 1 month ago (2016-08-19 03:10:26 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/618cb1f3e561b5d2a1dea9ec4653804f0da7...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b