calixteman
commented
1 year ago It's a regression from #16563. I filed a mirror bug on the Firefox bug tracker: https://bugzilla.mozilla.org/show_bug.cgi?id=1847733
Closed jsanahuja closed 1 year ago
calixteman
commented
1 year ago It's a regression from #16563. I filed a mirror bug on the Firefox bug tracker: https://bugzilla.mozilla.org/show_bug.cgi?id=1847733
calixteman
commented
1 year ago I read this bug report a bit too quickly and I overlooked the issue with the annotation storage.
calixteman
commented
1 year ago And when I try to print the document the rendering of the radio buttons is wrong.
calixteman
commented
1 year ago The annotation storage issue is because of:
P entry in the annotation which induces a data.page === -1 ;so the other fields are not correctly updated. In removing the page check, it's ok. @Snuffleupagus, do you remember why you added it (in #14023) ? do you think we could remove it safely ?
calixteman
commented
1 year ago An other possibility would be to correctly guess the page number in searching for the annotation in all the pages of the document.
Snuffleupagus
commented
1 year ago do you remember why you added it (in #14023) ?
This conversation feels quite familiar :-) Please see https://github.com/mozilla/pdf.js/issues/15096#issuecomment-1165814445
do you think we could remove it safely ?
It's probably safe, but does it actually fix the problem fully?
An other possibility would be to correctly guess the page number in searching for the annotation in all the pages of the document.
That'd require loading the entire document in order to parse just a single page, which is something that we go to great lengths to avoid in order to improve overall performance. Hence this is really not something that we could/should do unconditionally, it might be OK only as a fallback if it's the only way to fix things but you'd need to be very careful with the implementation.
calixteman
commented
1 year ago This conversation feels quite familiar :-) Please see #15096 (comment)
You've a better memory than mine...
do you think we could remove it safely ?
It's probably safe, but does it actually fix the problem fully?
I'd say yes. The goal of this function is to get annotation with the same name in order to update the storage: https://github.com/mozilla/pdf.js/blob/e914870c1410733e737a0184d1c748442ed92ed8/src/display/annotation_layer.js#L1595-L1597 and to update the visible annotations when the event comes from the sandbox: https://github.com/mozilla/pdf.js/blob/e914870c1410733e737a0184d1c748442ed92ed8/src/display/annotation_layer.js#L1615-L1619
I don't see any reasons to not get an annotation with page === -1: the annotation should be updated wherever they're.
About the JS side, the page property is readable and writable (in order to move an annotation from page to another).
For now, it's probably not dramatic to not have a correct page value even if it's a bug.
A pdf containing 100 pages with an annotation on page 1, an other one on page 100, both with no P entry and a JS script containing `getField("Foo1").value = getField("Foo100").page;" will be wrong whatever the annotations are and the only way to fix it would be to parse everything unfortunately.
jsanahuja
commented
1 year ago Thank you for your prompt response and review, guys.
I've also observed that the fields do not seem to be working correctly. The actual radio options are located within typeA15 object, whereas what I understand as the "no option selected" fields are within typeA15[x]:
await PDFViewerApplication.pdfDocument.getFieldObjects()
calixteman
commented
1 year ago The annotation storage issue is now fixed thanks to #16823. We still have an issue when printing:
Snuffleupagus
commented
1 year ago There also seems to be some problem with the scripting, since the following is logged when the document is loaded:
PDF 7fdd33f05987997a87811775215f56e8 [1.4 mPDF 8.0.0 / -] (PDF.js: 3.10.68 [0dd70c4e6]) [app.js:1594:12](webpack://pdf.js/web/app.js)
PDF.js Console:: q15_typeA15[0]q15_typeA15[0]q15_typeA15[0]q15_typeA15[0]q15_typeA15[1]q15_typeA15[1]q15_typeA15[1]q15_typeA15[1]q15_typeA15[2]q15_typeA15[2]q15_typeA15[2]q15_typeA15[2]q15_typeA15[3]q15_typeA15[3]q15_typeA15[3]q15_typeA15[3] [pdf_scripting_manager.js:292:18](webpack://pdf.js/web/pdf_scripting_manager.js)
TypeError: cannot set property 'display' of null
at <eval> (<input>:66)
at eval (native)
at globalEval (<initScript>)
at _runActions (<initScript>:2760)
at _initActions (<initScript>:2708)
at dispatch (<initScript>:2257)
at _dispatchEvent (<initScript>:1738)
at <anonymous> (<initScript>:196)
[pdf_scripting_manager.js:276:18](webpack://pdf.js/web/pdf_scripting_manager.js)Yep, I noticed that and there's the same error in Acrobat. The embedded js is buggy:
if (
this.app.viewerType == "Reader" ||
this.app.viewerType == "Exchange" ||
this.app.viewerType == "Exchange-Pro" || // Taken from: https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/js_api_reference.pdf (Page 96)
this.app.viewerType == "Foxit Reader"
) {
this.getField('adobeWarning').display = display.hidden;
this.getField('fakeSubmitButton').display = display.hidden;
} else {
this.getField('submitButton').display = display.hidden;
}and neither fakeSubmitButton nor submitButton exist.
Attach (recommended) or Link to PDF file here: https://github.com/mozilla/pdf.js/files/12290354/PDF.Form.4.pdf
Configuration:
Steps to reproduce the problem:
What is the expected behavior? (add screenshot) Being able to select one option per line (works in 3.6.126, not in 3.9.179)
Also,
PDFViewerApplication.pdfDocument.annotationStorage.getAll()should return only the selected ones.What went wrong? (add screenshot) You can only select one (not one per line), this does not happen in 3.6.126:
Return value of
PDFViewerApplication.pdfDocument.annotationStorage.getAll()is not right (I have selected all the options at least once, but the result is that all are selected). This also happens in 3.6.126As far as I can see, the PDF is not wrong:
Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):