XFA: Fix a crashier in SetFocus()

XFA: Fix a crashier in SetFocus()

BUG=pdfium:374
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/c30d5bf3efaef5c7254ae326466e6a35cff63365

[jun_fang, 2016-01-28 11:16:06]

jun_fang@foxitsoftware.com changed reviewers:
+ thestig@chromium.org, tsepez@chromium.org

[jun_fang, 2016-01-28 11:16:06]

Hi Tom and Lei,

Please help to review this CL. It needs another patch in chromium side. I will
provide it later. Thanks!

[jun_fang, 2016-01-28 11:23:20]

On 2016/01/28 11:16:06, jun_fang wrote:
> Hi Tom and Lei,
> 
> Please help to review this CL. It needs another patch in chromium side. I will
> provide it later. Thanks!

The patch in chromium is ready at https://codereview.chromium.org/1643943002/.

[Tom Sepez, 2016-01-28 17:30:25]

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:381: *  without the XFA module) or must be 2 (when built
with the XFA module).
Do we need to change the version number here to account for the API change?

[jun_fang, 2016-01-29 00:19:18]

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:381: *  without the XFA module) or must be 2 (when built
with the XFA module).
On 2016/01/28 17:30:25, Tom Sepez wrote:
> Do we need to change the version number here to account for the API change?

Yes. I forgot to check the description. I will change the API version to 2 in
the next patch.

[jun_fang, 2016-01-29 12:58:45]

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1640233004/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:381: *  without the XFA module) or must be 2 (when built
with the XFA module).
On 2016/01/29 00:19:18, jun_fang wrote:
> On 2016/01/28 17:30:25, Tom Sepez wrote:
> > Do we need to change the version number here to account for the API change?
> 
> Yes. I forgot to check the description. I will change the API version to 2 in
> the next patch.

The comments here are used to remind applications to set a correct version like:

#ifdef PDF_ENABLE_XFA
  formfillinfo->version = 2;
#else   // PDF_ENABLE_XFA
  formfillinfo->version = 1;
#endif  // PDF_ENABLE_XFA

I update interface version to 2 but it doesn't change any behaviors.

[Tom Sepez, 2016-01-29 16:47:11]

Since XFA is still "beta", I think its fine to leave the version at 2, rather
than bumping it to 3. LGTM

https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:712: *           2
Thanks for correcting these comments.

[jun_fang, 2016-01-29 16:55:09]

On 2016/01/29 16:47:11, Tom Sepez wrote:
> Since XFA is still "beta", I think its fine to leave the version at 2, rather
> than bumping it to 3. LGTM
> 
> https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h
> File public/fpdf_formfill.h (right):
> 
>
https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h#...
> public/fpdf_formfill.h:712: *           2
> Thanks for correcting these comments.

Please review the pair patch at https://codereview.chromium.org/1643943002/.

[jun_fang, 2016-02-01 04:33:35]

Patchset #4 (id:60001) has been deleted

[jun_fang, 2016-02-01 04:33:52]

Patchset #4 (id:80001) has been deleted

[jun_fang, 2016-02-01 05:18:52]

On 2016/01/29 16:55:09, jun_fang wrote:
> On 2016/01/29 16:47:11, Tom Sepez wrote:
> > Since XFA is still "beta", I think its fine to leave the version at 2,
rather
> > than bumping it to 3. LGTM
> > 
> > https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h
> > File public/fpdf_formfill.h (right):
> > 
> >
>
https://codereview.chromium.org/1640233004/diff/40001/public/fpdf_formfill.h#...
> > public/fpdf_formfill.h:712: *           2
> > Thanks for correcting these comments.
> 
> Please review the pair patch at https://codereview.chromium.org/1643943002/.

Please review patch 4. In this patch, I removed the definition of FFI_PageEvent
from _FPDF_FORMFILLINFO.

[jun_fang, 2016-02-03 02:12:52]

Description was changed from

==========
XFA: Fix a crashier in SetFocus()

BUG=pdfium:374
==========

to

==========
XFA: Fix a crashier in SetFocus()

BUG=pdfium:374
R=tsepez@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/c30d5bf3efaef5c7254ae326466e6a35cff6...
==========

[jun_fang, 2016-02-03 02:12:58]

Committed patchset #4 (id:100001) manually as
c30d5bf3efaef5c7254ae326466e6a35cff63365 (presubmit successful).