Bug 114185 - 502 err in master but not in LO 5.3.2.1 to 5.4.4
Summary: 502 err in master but not in LO 5.3.2.1 to 5.4.4
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha1+
Hardware: All All
: high normal
Assignee: Eike Rathke
URL:
Whiteboard: target:6.1.0 target:6.0.0.1
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2017-11-30 19:40 UTC by Xavier Van Wijmeersch
Modified: 2017-12-15 14:30 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
502 err in LO master alpha and beta (54.88 KB, application/vnd.oasis.opendocument.spreadsheet)
2017-11-30 19:40 UTC, Xavier Van Wijmeersch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xavier Van Wijmeersch 2017-11-30 19:40:37 UTC
Created attachment 138157 [details]
502 err in LO master alpha and beta

The formula in A1 and A46 give me a 502 error with

Version: 6.0.0.0.beta1+
Build ID: e238216f4a8344498180731b99c6e6de01947da5
CPU threads: 8; OS: Linux 4.14; UI render: default; VCL: kde4; 
Locale: nl-BE (en_US.UTF-8); Calc: group threaded

Version: 6.1.0.0.alpha0+
Build ID: 897c07bda2d116bcc2fa4a64c1eb75a52651c991
CPU threads: 8; OS: Linux 4.14; UI render: default; VCL: kde4; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2017-11-26_15:33:58
Locale: nl-BE (en_US.UTF-8); Calc: group threaded

but not with

Version: 5.3.1.2
Build ID: e80a0e0fd1875e1696614d24c32df0f95f03deb2
CPU Threads: 8; OS Version: Linux 4.14; UI Render: default; VCL: kde4; Layout Engine: new; 
Locale: nl-BE (en_US.UTF-8); Calc: group

Version: 5.3.8.0.0+
Build ID: 7f1297d9b4f449eb9ada8008fb21b7046d1a8f19
CPU Threads: 8; OS Version: Linux 4.14; UI Render: default; VCL: kde4; Layout Engine: new; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:libreoffice-5-3, Time: 2017-11-10_15:56:34
Locale: nl-BE (en_US.UTF-8); Calc: group

Version: 5.4.4.0.0+
Build ID: e3dc0612e1e75e53c0483755c6dc2d47db938c91
CPU threads: 8; OS: Linux 4.14; UI render: default; VCL: kde4; 
Locale: nl-BE (en_US.UTF-8); Calc: group
Comment 1 Xisco Faulí 2017-11-30 21:47:22 UTC
I get the 502 error in

Version: 6.1.0.0.alpha0+
Build ID: 2618e4a13d719122e6358d9d96864d1691e56a02
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: th-TH (ca_ES.UTF-8); Calc: group threaded

Version: 5.4.2.2
Build ID: 1:5.4.2~rc2-0ubuntu0.17.04.1~lo2
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: es-ES (ca_ES.UTF-8); Calc: group

Version: 5.2.0.0.alpha1+
Build ID: 5b168b3fa568e48e795234dc5fa454bf24c9805e
CPU Threads: 4; OS Version: Linux 4.10; UI Render: default; 
Locale: ca-ES (ca_ES.UTF-8)

Did you attach the correct file?
Comment 2 Aron Budea 2017-12-01 06:09:43 UTC
Locale has to be set to Dutch (Netherlands) for repro.
Bibisected to the commit referenced below using repo bibisect-win32-6.0. Adding CC: to Laurent BP, please take a look sometimes.

author		Laurent BP <laurent.balland-poirier@laposte.net>	2017-10-08 20:05:34 +0200
committer	Eike Rathke <erack@redhat.com>	2017-10-19 22:13:44 +0200
commit 80c0a7300b9e185cd77f754abbad31422826662c (patch)
tree 242ddf9686f575c4e6fd463f6f061eb73001c170
parent 334a9f16cd1d1f9694f885c759903a41aa3d4833 (diff)
tdf#33689 Accept English syntax keywords in format strings
Comment 3 Xavier Van Wijmeersch 2017-12-01 09:40:44 UTC
@Xisco Fauli

Yes, i did attach the correct file. Its one i work daily with.
Comment 4 Laurent Balland 2017-12-01 12:29:31 UTC
I see the problem. Number format "#,## Kwh" is understandable in nl because h has no meaning (like da, no, sv, and fi), so Kwh is treated as text. Whereas in other languages h is treated as hour.
A quick workaround is to use a format with \h instead of h to be sure to treat it as text.

I will have a look to see how we can fix correctly bug 33689 without messing languages using coding character for number format.
Comment 5 m_a_riosv 2017-12-01 23:48:58 UTC
If I'm not wrong text in format must be introduced inside double quotes or escaped as @Laurent comment.
LibreOffice help
"
Including Text in Number Format Codes / Text and Numbers
To include text in a number format that is applied to a cell containing numbers, place a double quotation mark (") in front of and behind the text, or a backslash (\) before a single character. For example, enter #.# "meters" to display "3.5 meters" or #.# \m to display "3.5 m". If you use space as thousands separator, you need to insert spaces between quotes in the previous examples: #.#" meters" or #.#\ \m to get the correct result.
"

So I think the formula it's out of rules, but can be escaping the text "#,##  \K\w\h", or the double quotes "#,##""  Kwh""", or as it's a formula putting the text out of the function TEXT(;)&"  Kwh" what seems to me the simplest and safe.

Hope that avoiding rules won't carry other issues.
Comment 6 Laurent Balland 2017-12-02 00:59:57 UTC
(In reply to m.a.riosv from comment #5)
> If I'm not wrong text in format must be introduced inside double quotes or
> escaped as @Laurent comment.
Actually, "text in format *should* be introduced inside double quotes". The rule is permissive and accept text without quotes if there is no ambiguity: it must not contain any format pattern. But format patterns depend on locale.

Resolution of bug 33689 makes accept English patterns in any locale, English patterns are then now forbidden in text without quotes. This will impact 10 locales, with following patterns (upper and lowercase):
- de: YY, D
- nl: YY, H
- fr: YY, D, NN
- it: YY, D, NN
- pt and es: YY, NN
- da, no, nb, nn and sv: H
- fi: YY, M, D, H
Comment 7 Laurent Balland 2017-12-05 20:43:10 UTC
(In reply to Laurent BP from comment #6)
> Resolution of bug 33689 makes accept English patterns in any locale, English
> patterns are then now forbidden in text without quotes. This will impact 10
> locales, with following patterns (upper and lowercase):
> - de: YY, D
> - nl: YY, H
> - fr: YY, D, NN
> - it: YY, D, NN
> - pt and es: YY, NN
> - da, no, nb, nn and sv: H
> - fi: YY, M, D, H

If you used a number format with text without quotes, when save and reload, quotes are automatically added.

So this regression could only appear if TEXT() function is used to apply a number format build from a string. A resolution need first to detect the problem, with all these conditions:
- if TEXT() is used
- if file was saved with a version prior 6.0
- if local is one of de, nl, fr, it, pt, es, da, no, nb, nn, sv, fi
- if one of the English strings indicated above (limited to the locale) is detected in the format string
=> Analyze the complete format string to detect if this English string is inside quotes or after a \

If no, what should we do?
- nothing as right now => function returns a 502 error (Invalid argument) as format is invalid
- advertise user that he/she *must* quote his/her text in number format
- modify the string format to silently insert quotes (like it is done in a number format)

Note that fi is quite complicated, M could stand for minute in fi.

Another total different way to fix this, would be to only activate English number format with an option, which will be off as default.
Comment 8 Eike Rathke 2017-12-12 14:18:43 UTC
@Laurent: sorry for taking this away from you, but I have a promising approach how to solve this.
Comment 9 Laurent Balland 2017-12-12 15:07:16 UTC
(In reply to Eike Rathke from comment #8)
> @Laurent: sorry for taking this away from you, but I have a promising
> approach how to solve this.

@Eike: I should have retired as I was no more working on it.
Comment 10 Commit Notification 2017-12-12 20:16:26 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=482182fd0331a0be371f8f58dc74aa05a68f0a1d

Prepare ImpSvNumberformatScan with KeywordLocalization context, tdf#114185

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2017-12-12 20:16:34 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b2cd9dd31183c45942c2f522104654ff1f8e6889

Resolves: tdf#114185 force KeywordLocalization::LocaleLegacy

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Eike Rathke 2017-12-12 20:36:06 UTC
@Xavier: note that while this fixes the case for loading that document in the nl-NL locale, it did and will fail in any locale that uses one of the Kwh characters as keyword. Or, fwiw, in locales that use a different decimal separator than ',' comma. To fix that properly enquote the "Kwh" string and set the cell's number format's locale explicitly to "Dutch (Netherlands)" (not "Default - Dutch (Netherlands)").
Comment 13 Commit Notification 2017-12-13 11:44:34 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4e34b003bc8908e0f9f78e8ed5e84e7a5e42a6ca&h=libreoffice-6-0

Prepare ImpSvNumberformatScan with KeywordLocalization context, tdf#114185

It will be available in 6.0.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2017-12-13 11:44:42 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c1293b4fa344096b4de6b8648a0409ca5811eccc&h=libreoffice-6-0

Resolves: tdf#114185 force KeywordLocalization::LocaleLegacy

It will be available in 6.0.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Xavier Van Wijmeersch 2017-12-13 12:39:48 UTC
I just did tested and its working. Thank you Eike.

Version: 6.1.0.0.alpha0+
Build ID: 6ca6d6ac912588a8f62d7e6b668ebec333752ebc
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: kde4; 
Locale: nl-BE (en_US.UTF-8); Calc: group threaded
Comment 16 Xavier Van Wijmeersch 2017-12-14 09:46:06 UTC
works also with

Version: 6.0.0.0.beta2+
Build ID: c1293b4fa344096b4de6b8648a0409ca5811eccc
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: kde4; 
Locale: nl-BE (en_US.UTF-8); Calc: group threaded
Comment 17 Eike Rathke 2017-12-15 14:30:53 UTC
Let's set to verified then. And thanks for testing :-)