Bug 134455 - Timevalue Err:502 REGRESSION
Summary: Timevalue Err:502 REGRESSION
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.4.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:7.1.0 target:7.0.0.2 target:6.4.6
Keywords: bibisected, bisected
Depends on:
Blocks:
 
Reported: 2020-07-02 02:21 UTC by aplatypus
Modified: 2020-08-06 08:01 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample spreadsheet to reproduce the bug (5.16 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2020-07-02 02:24 UTC, aplatypus
Details
Screenshot of error using: v6.4.3. (40.02 KB, image/png)
2020-07-02 02:25 UTC, aplatypus
Details
Screenshot of correct behaviour using: v6.3.4.2. (44.20 KB, image/png)
2020-07-02 02:26 UTC, aplatypus
Details
Comparison MSO 2010 and LibreOffice 7.1 master (66.82 KB, image/png)
2020-07-02 09:01 UTC, Xisco Faulí
Details

Note You need to log in before you can comment on or make changes to this bug.
Description aplatypus 2020-07-02 02:21:53 UTC
Description:
Timevalue function produces "Err:502" on a long used and established formula.  

* This forula has not changed since 2015

* It is used to convert a number in minutes to a time value
    x Example: =TIMEVALUE("0:66") --> used to give "01:06" (one hour and 6 minutes).

* A spreadsheet included that works: v6.3.4.2, fails: v6.4.3.2

* The required result is a time value that is used in other time related formulas.

* The error itself fails to provide an number time value.  For example time for 5 minutes:  =TIMEVALUE("0:05") -> "00:05" or 0.003472 (numeric)
    x No numeric version for:  =TIMEVALUE("0:66") is available, just the error string.
        

Steps to Reproduce:
See attached example.

1.  Enter formula ' =TIMEVALUE("0:66")' should give 1 hour and 6 minutes
2.  So on for similar time values
3.  Foruma must give a decimal value suitable for use in further time caculations.

Actual Results:
The resulting value is:

Err:502


Expected Results:
The correctly formatted time text string must be converted to a the corresponding valid time value


Reproducible: Always


User Profile Reset: No


OpenGL enabled: Yes

Additional Info:

This is a SHOW STOPPER.  I must UNINSTALL the release version v6.4.3.

I must revert v6.3.4.2.

We use probably millions of time calculations in Excel or LibreOffice.  

This bug is a really serious Q/A failure.

LibreOffice About box included with the screen shots.

I hae not found out how to provide uploads/screenshots and sample spreadsheet.
Comment 1 aplatypus 2020-07-02 02:24:46 UTC
Created attachment 162579 [details]
Sample spreadsheet to reproduce the bug
Comment 2 aplatypus 2020-07-02 02:25:51 UTC
Created attachment 162580 [details]
Screenshot of error using: v6.4.3.
Comment 3 aplatypus 2020-07-02 02:26:38 UTC
Created attachment 162581 [details]
Screenshot of correct behaviour using:  v6.3.4.2.
Comment 4 Mike Kaganski 2020-07-02 04:29:48 UTC
Confirmed in Version: 6.4.5.2 (x64)
Build ID: a726b36747cf2001e06b58ad5db1aa3a9a1872d6
CPU threads: 12; OS: Windows 10.0 Build 18363; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: CL

OASIS OpenDocument 6.1.18 tells:

> TIMEVALUE
> Summary: Returns a time serial number from given text.
> 
> Syntax: TIMEVALUE( Text T )
> 
> Returns: Time
> 
> Constraints: None
> 
> Semantics: This computes the serial number of the text string T, which is a time,
> using the current locale. This function shall accept ISO time format (HH:MM:SS),
> which is locale-independent. If the text of T has a combined date and time format,
> e.g. YYYY-MM-DD HH:MM:SS, the fractional part of the date serial number is returned.
> If the text of T does not have a time format, an evaluator may attempt to convert
> the number another way (e.g., using VALUE), or it may return an Error (this is
> implementation-dependent).
> 
> See also TIME 6.10.17, DATE 6.10.2, DATEVALUE 6.10.4, VALUE 6.13.34

As I read it, it accepts *a* time format (see the "If the text of T does not have a time format"), so the requirement to accept ISO time format is not exclusive, but a minimal requirement, not denying accepting other formats like non-ISO "0:66". Additionally, there's an explicit provision that the value may be converted using *another* (unspecified) way (and an example given, which again is not exclusive).

This issue creates interoperability problem, since e.g. MS Excel 2016 accepts such values, thus formulas working in Excel don't work in Calc.
Comment 5 Xisco Faulí 2020-07-02 09:01:38 UTC
Created attachment 162586 [details]
Comparison MSO 2010 and LibreOffice 7.1 master
Comment 6 Xisco Faulí 2020-07-02 09:06:09 UTC
The issue started to happen after

https://cgit.freedesktop.org/libreoffice/core/commit/?id=7d72b9d34c1183b7471a7a97c007aba10de2d27e

author	Eike Rathke <erack@redhat.com>	2019-10-18 19:35:47 +0200
committer	Eike Rathke <erack@redhat.com>	2019-10-18 23:44:09 +0200
commit 7d72b9d34c1183b7471a7a97c007aba10de2d27e (patch)
tree f4f1b1a5a2a5c184626f3efef83277416c157c4f
parent b3f67f9887e30240292b2f40c64161ed53281795 (diff)
Input with subsequent part greater than 59 is not time or duration

Not sure whether it's a regression, since the commit changed value greater than 59 not to be time, however the value is displayed correctly in MSO 2010
Comment 7 Mike Kaganski 2020-07-02 09:28:10 UTC
(In reply to Xisco Faulí from comment #6)
> Not sure whether it's a regression, since the commit changed value greater
> than 59 not to be time, however the value is displayed correctly in MSO 2010

Of course it is not; and of course the original description is over-optimistic declaring "0:66" a "correctly formatted time text string". But yes, it's an interoperability issue (that might need a fix on a specific function level, to avoid treating invalid times in general, but still allow in the TIMEVALUE function where they are told explicitly to be times).
Comment 8 Eike Rathke 2020-07-02 11:03:27 UTC
TIMEVALUE() uses the number parser's input detection, we maybe could tell it to lax it in this case and accept the dirt the user throws at it as being intentional.
Comment 9 Eike Rathke 2020-07-02 11:12:24 UTC
Fwiw, processing "0:66" as time value is possible with
=VALUE(RIGHT("0:66";2))/24/60
Comment 10 Commit Notification 2020-07-02 21:54:59 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/f106bb4471b6d3627311980ed727459b2da8e448

Resolves: tdf#134455 Let TIMEVALUE() use lax time recognition

It will be available in 7.1.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 11 Eike Rathke 2020-07-02 21:57:22 UTC
Pending
CI for 7-0 https://gerrit.libreoffice.org/c/core/+/97792
review for 6-4 https://gerrit.libreoffice.org/c/core/+/97793
Comment 12 Commit Notification 2020-07-02 23:33:17 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/30ea3ba86b110db68c19815c5c67d4d315c091bf

Resolves: tdf#134455 Let TIMEVALUE() use lax time recognition

It will be available in 7.0.0.2.

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

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2020-07-02 23:57:39 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/07987dc9aefdd48010edd0934dde5239c0dc8019

Resolves: tdf#134455 Let TIMEVALUE() use lax time recognition

It will be available in 6.4.6.

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

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2020-07-03 16:08:11 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/643df295fa72b0d252460ae98104e347ce76cc22

tdf#134455: sc: Add unittest

It will be available in 7.1.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 15 aplatypus 2020-07-04 14:39:30 UTC
* Just to  add this bug is in current Release v6.4.5

* Speaking as a user - This level of error needs to be patched in the current Release version.

* This is a very critical function to work-around limitations in the time calculations system.
Comment 16 aplatypus 2020-07-05 02:25:16 UTC
Hi -- Works on my PC v7 3rd July 2020 current.

I just tested the fix on my PC, where I first saw the problem.
Comment 17 Eike Rathke 2020-07-06 06:53:57 UTC
(In reply to aplatypus from comment #15)
> * Just to  add this bug is in current Release v6.4.5
See comment 13.
Comment 18 egc 2020-08-01 10:05:11 UTC
I just tested it on LO 6.4.6 for Linux

Typing 0:66 still give the text 0:66 instead of the time value 01:06
Comment 19 Buovjaga 2020-08-02 13:52:42 UTC
(In reply to egc from comment #18)
> I just tested it on LO 6.4.6 for Linux
> 
> Typing 0:66 still give the text 0:66 instead of the time value 01:06

You mean you

1. Formatted a cell to use Time number format
2. Inputted =TIMEVALUE("0:66")

and it did not show 01:06:00 ?

Works for me with master.

Arch Linux 64-bit
Version: 7.1.0.0.alpha0+
Build ID: 869ef9f2118158222032964a5b24ae7d760b84e9
CPU threads: 8; OS: Linux 5.7; UI render: default; VCL: kf5
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded
Built on 31 July 2020
Comment 20 egc 2020-08-02 23:07:12 UTC
(In reply to Buovjaga from comment #19)
> (In reply to egc from comment #18)
> > I just tested it on LO 6.4.6 for Linux
> > 
> > Typing 0:66 still give the text 0:66 instead of the time value 01:06
> 
> You mean you
> 
> 1. Formatted a cell to use Time number format
> 2. Inputted =TIMEVALUE("0:66")
> 
> and it did not show 01:06:00 ?
> 
Not typing =TIMEVALUE("0:66"), just typing 0:66, like it was always possible before. Why was this handy behaviour changed at all?

Typing =TIMEVALUE("0:66") doesn't make any sense, then i'm a lot faster with calculating and typing 1:06 ...
Comment 21 Buovjaga 2020-08-03 06:16:09 UTC
(In reply to egc from comment #20)
> (In reply to Buovjaga from comment #19)
> > (In reply to egc from comment #18)
> > > I just tested it on LO 6.4.6 for Linux
> > > 
> > > Typing 0:66 still give the text 0:66 instead of the time value 01:06
> > 
> > You mean you
> > 
> > 1. Formatted a cell to use Time number format
> > 2. Inputted =TIMEVALUE("0:66")
> > 
> > and it did not show 01:06:00 ?
> > 
> Not typing =TIMEVALUE("0:66"), just typing 0:66, like it was always possible
> before. Why was this handy behaviour changed at all?
> 
> Typing =TIMEVALUE("0:66") doesn't make any sense, then i'm a lot faster with
> calculating and typing 1:06 ...

Ok, it's true that commit 7d72b9d34c1183b7471a7a97c007aba10de2d27e affected this as well, I just tested.
Comment 22 Mike Kaganski 2020-08-03 07:53:54 UTC
(In reply to egc from comment #20)

This issue is specifically about TIMEVALUE. The change in https://git.libreoffice.org/core/+/7d72b9d34c1183b7471a7a97c007aba10de2d27e of course affected all cases, and this issue restored previous behavior in TIMEVALUE, where it's explicitly clear that time is being asked, even if it is 
invalid.

This one is fixed. There is tdf#135249 for what you are talking about.
Comment 23 egc 2020-08-06 08:01:00 UTC
(In reply to Mike Kaganski from comment #22)
> (In reply to egc from comment #20)
> 
> This one is fixed. There is tdf#135249 for what you are talking about.

ok, thanks Mike for the link!