Bug 474661 - Norwegian withholding tax form - wrong calculation of percent value
Summary: Norwegian withholding tax form - wrong calculation of percent value
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: PDF backend (show other bugs)
Version: 23.08.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-18 16:49 UTC by andreaswuest
Modified: 2024-09-16 12:28 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 24.08.0
Sentry Crash Report:


Attachments
various input field combinations showing the error. (66.92 KB, image/png)
2023-09-18 16:49 UTC, andreaswuest
Details
PDF document with the problem. (627.50 KB, application/pdf)
2023-09-18 16:50 UTC, andreaswuest
Details

Note You need to log in before you can comment on or make changes to this bug.
Description andreaswuest 2023-09-18 16:49:44 UTC
Created attachment 161696 [details]
various input field combinations showing the error.

STEPS TO REPRODUCE
1.  Open the attached document. 
2. Go to the second page and enter the value "133.34" in "gross amount in NOK field" 

OBSERVED RESULT
Withholding tax 25% is calculated not properly => 33,335.00

EXPECTED RESULT
Should be 33.33

The same applies to the value 133.35 -> the 10% value is not properly calculated.
If i enter 133.33 the value is calcluated properly for 25% and 10%


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Tuxedo OS
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 5.109.0
Qt Version: 5.15.10

ADDITIONAL INFORMATION
Comment 1 andreaswuest 2023-09-18 16:50:18 UTC
Created attachment 161697 [details]
PDF document with the problem.
Comment 2 Volker Krause 2023-09-26 17:23:12 UTC
This seems to happen only in locales with a comma as decimal separator. It breaks when the string "33.335" is passed to JSUtil::stringToNumber(), which results in the number 33335 in a locale with a comma decimal separator, rather than the expected 33.335, ie. this happens for numbers where the conversion is ambiguous between different locales.
Comment 3 andreaswuest 2023-10-05 18:51:37 UTC
Here the sake of completeness - my kde sponsored work entry: 

https://discuss.kde.org/t/bug-fix-fixing-okular-js-form-calculation-problems/5495
Comment 4 andreaswuest 2023-10-05 19:43:36 UTC
The strange thing is also, that it is not consistent

133.33 -> both 25% and 10% are properly calculated
133.34 -> 25% is calculated wrong
133.35 -> 10% is calculated wrong
133.36 -> both 25% and 10% are properly calculated

=> The decimal separator and the thousand separator are displayed properly when you just enter "1000" and leave the field. 
Why the calculation does not work for some numbers i do not understand.
Comment 5 Volker Krause 2023-10-06 15:57:37 UTC
It only triggers when the result has exactly three decimals, as only that produces a string that can be misinterpreted as a value with a dot as group separator instead of decimal separator.

This probably isn't even terribly hard to fix, the problem is I am not finding the involved methods in the PDF JS API spec to know how exactly those are supposed to behave.
Comment 6 Albert Astals Cid 2023-10-06 20:01:01 UTC
Are you asking about sepStyle in AFNumber_Format?

According to https://acrobatusers.com/forum/javascript/setting-number-format-text-field/

sepStyle = separator style 
  0 = 1,234.56
  1 = 1234.56
  2 = 1.234,56
  3 = 1234,56
Comment 7 Volker Krause 2023-10-06 20:41:25 UTC
(In reply to Albert Astals Cid from comment #6)
> Are you asking about sepStyle in AFNumber_Format?

The output part of that is clear I think (and the implementation matches that), the question is how the input should be interpreted in case of that being a string rather than a number. The current implementation is using the current locale format first, and then the alternate format. Unconditionally using the English format would probably fix this, but the current logic presumably didn't happen by accident, so there must be more to this.
Comment 8 Albert Astals Cid 2023-10-06 22:34:19 UTC
The internet says it is a localized number

pdfium and pdf.js say "it doesn't matter, just replace the , to . and parse that" (with the understanding that there's no thousand separators i guess)
Comment 9 Bug Janitor Service 2023-10-07 10:54:38 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/832
Comment 10 Volker Krause 2023-10-07 10:55:50 UTC
(In reply to Albert Astals Cid from comment #8)
> The internet says it is a localized number
> 
> pdfium and pdf.js say "it doesn't matter, just replace the , to . and parse
> that" (with the understanding that there's no thousand separators i guess)

Indeed, assuming there are no group separators for all string to number conversions fixes this.
Comment 11 Nicolas Fella 2023-12-11 19:26:56 UTC
The form field has a AFNumber_Keystroke(2, 0, 0, 0, "", true) keystroke script (that Okular doesn't implement, which is another bug in itself)

This means . is decimal and , is thousand separator.

Acrobat and Firefox don't even let me type in a comma, Chromium treats it as a decimal separator.
Comment 12 Bug Janitor Service 2024-01-16 18:04:36 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/914
Comment 13 Bug Janitor Service 2024-05-19 06:07:00 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/992
Comment 14 Albert Astals Cid 2024-07-09 22:57:20 UTC
Git commit 0520b64fed2d0b61a81740f164a4ae573a6aeb32 by Albert Astals Cid, on behalf of Pratham Gandhi.
Committed on 09/07/2024 at 22:57.
Pushed by aacid into branch 'master'.

Fix numerical interpretation in form fields

With the previous implementations of AFNumber_Keystroke [MR 988](https://invent.kde.org/graphics/okular/-/merge_requests/988), we ensure that during a keystroke event, a user does not enter thousands separator. With this in mind, numerical fields can always be first converted to en_US format and then used for calculations. This ensures that all numbers are correctly interpreted.

M  +1    -1    autotests/calculatetexttest.cpp
M  +1    -1    autotests/parttest.cpp
M  +10   -6    core/script/builtin.js
M  +4    -2    core/script/js_field.cpp

https://invent.kde.org/graphics/okular/-/commit/0520b64fed2d0b61a81740f164a4ae573a6aeb32