Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong value when loading field values from annotationStorage #17046

Closed
jsanahuja opened this issue Sep 28, 2023 · 8 comments
Closed

Wrong value when loading field values from annotationStorage #17046

jsanahuja opened this issue Sep 28, 2023 · 8 comments

Comments

@jsanahuja
Copy link

Attach (recommended) or Link to PDF file here:
test_form.pdf

Configuration:

  • Web browser and its version: Google Chrome 117.0.5938.92
  • Operating system and its version: Windows 10 Business 10.0.19045 19045
  • PDF.js version: 3.9.179
  • Is a browser extension: No

Steps to reproduce the problem:

  1. Edit viewer.html and add the following code to set values in annotationStorage before rendering.
<script>
  (function(){
    var customValues = {
      '12R': true,
      '13R': 'Test',
      '15R': true,
      '16R': false,
      '17R': false,
      '19R': 'Item4L',
      '21R': 'Item2'
    };
    document.addEventListener("webviewerloaded", () => {
      PDFViewerApplication.initializedPromise.then(() => {
        PDFViewerApplication.eventBus.on('pagesloaded', (e) => {
          for (let key in customValues) {
            PDFViewerApplication.pdfDocument.annotationStorage.setValue(key, { value: customValues[key] });
          }
        });
      });
    });
  })();
</script>
  1. Open viewer and see all fields have the loaded value but text field.

What is the expected behavior? (add screenshot)
(Ignore field background colors, that was set for testing)
Set the value from annotationStorage
image

What went wrong? (add screenshot)
(Ignore field background colors, that was set for testing)
Text field does not load correctly annotationStorage value:
image
On focus it does set the correct value but if I do not change it, sets back the default value on blur
image

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

@Snuffleupagus
Copy link
Collaborator

PDF.js version: 3.9.179

That version is no longer supported, please find the latest releases at https://mozilla.github.io/pdf.js/getting_started/#download

@jsanahuja
Copy link
Author

jsanahuja commented Sep 28, 2023

Note that in the example customValues are hardcoded but in our application we store customValues getting them from annotationStorage.getValue(id, {}) (we do this to keep values after leaving the page) and it does return {value: 'test', formattedValue: null}.

After checking render method of TextWidgetAnnotationElement I have tried setting customValues as follows:

customValues[id] = annotationStorage.getValue(id, {})
customValues[id].formattedValue = customValues[id].value;

And works just fine

PS: I will try in the latest version soon

@calixteman
Copy link
Contributor

A text field has 2 values: the one entered by the user, e.g. 123 (userValue or value), and the one displayed when the field is unfocused, e.g. 123,00€ (formattedValue).

@jsanahuja
Copy link
Author

jsanahuja commented Sep 28, 2023

A text field has 2 values: the one entered by the user, e.g. 123 (userValue or value), and the one displayed when the field is unfocused, e.g. 123,00€ (formattedValue).

Shouldn't then annotationStorage.getValue(id, {}) include the real formattedValue? It returns {value: 'test', formattedValue: null}.

If no, setting annotationStorage.setValue(id, {value: 'test', formattedValue: null}) before rendering still shows the default value, which I believe it's no correct

@calixteman
Copy link
Contributor

Yes maybe... formattedValue === null just means that nothing set it (usually an embedded js code can format the string). So it's up to you to guess that in such a case the formatted value is the value or maybe you could do something else with this information... I don't know.
That said, if you don't have a real bug in the Firefox pdf viewer or at https://mozilla.github.io/pdf.js/web/viewer.html, we won't fix anything (except of course if something is obviously completely wrong). Here I've the impress it's just a detail and likely a matter of taste.

@jsanahuja
Copy link
Author

jsanahuja commented Sep 28, 2023

Yes maybe... formattedValue === null just means that nothing set it (usually an embedded js code can format the string). So it's up to you to guess that in such a case the formatted value is the value or maybe you could do something else with this information... I don't know.

Then I would say value should have more "priority" than default value. That is:

let fieldFormattedValues =
storedData.formattedValue || this.data.textContent?.join("\n") || null;
Should be:

let fieldFormattedValues =
        storedData.formattedValue || storedData.value || this.data.textContent?.join("\n") || null;

And these both

element.textContent = fieldFormattedValues ?? textContent;
element.setAttribute("value", fieldFormattedValues ?? textContent);
simply use fieldFormattedValues

That said, if you don't have a real bug in the Firefox pdf viewer or at https://mozilla.github.io/pdf.js/web/viewer.html, we won't fix anything (except of course if something is obviously completely wrong). Here I've the impress it's just a detail and likely a matter of taste.

I don't think this could be reproduced in Firefox PDF Viewer. If you don't think this has to be changed I think I can go with setting formattedValue with value if formattedValue === null

@calixteman
Copy link
Contributor

this.data.textContent is built from the appearance stream (the one you've when you print a form for example) so it's a formatted value and has the precedence over value.

@jsanahuja
Copy link
Author

I believe that default values (formatted or not) shouldn't take precedence over annotationStorage values. It would be more intuitive if they behaved like user input values.

For example, on user input:

element.addEventListener("input", event => {
storage.setValue(id, { value: event.target.value });
this.setPropertyOnSiblings(
element,
"value",
event.target.value,
"value"
);
elementData.formattedValue = null;
});
elementData.formattedValue is set to null to prevent exactly this (in case formattedValue is not updated by the PDF)

Anyway, I will go with setting formattedValue myself but thank you for your prompt responses and the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants