Bug 54651 - PIVOTTABLE time-format gone
Summary: PIVOTTABLE time-format gone
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.6.0.0.beta1
Hardware: Other All
: high normal
Assignee: Kohei Yoshida
URL:
Whiteboard: bibisected36a target:4.1.0 target:4.0...
Keywords: regression
Depends on:
Blocks:
 
Reported: 2012-09-07 19:39 UTC by ktos.obcy+freedesktop
Modified: 2014-02-23 05:43 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
pivot_table_bug (58.27 KB, application/vnd.oasis.opendocument.spreadsheet)
2012-09-07 19:39 UTC, ktos.obcy+freedesktop
Details
bibisect36a (1.18 KB, text/plain)
2012-10-16 18:40 UTC, Joel Madero
Details
formatting of the pivot table is determined by the last row content (10.51 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-02-07 15:42 UTC, ktos.obcy+freedesktop
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ktos.obcy+freedesktop 2012-09-07 19:39:32 UTC
Created attachment 66809 [details]
pivot_table_bug

Open attachment, refresh pivot table at the bottom of the table.
Starting from 3.6.x Libre Office Calc stopped displaying correctly time column and instead of having time summed I have regular numbers/fractions. Same spreadsheet works ok in the previous versions of LO as well as refreshing document produced in 3.6 returns to the proper form.
Comment 1 business.coaching 2012-09-26 02:17:30 UTC
Calculations are well-performed. Displaying the time format is defective.
Comment 2 Joel Madero 2012-10-15 00:27:28 UTC
Verified. Since you said it used to work I'm going to add Regression to whiteboard.


Leaving as normal because of the regression (otherwise I'd say minor). 

Marking as HIGH as it's a regression.
Comment 3 Joel Madero 2012-10-16 18:40:55 UTC
Created attachment 68641 [details]
bibisect36a
Comment 4 ktos.obcy+freedesktop 2012-12-05 13:19:10 UTC
Just tested it with the latest 3.6.4 and the bug is still there. I thought that it might be fixed by the https://bugs.freedesktop.org/show_bug.cgi?id=54898 but it wasn't.
Comment 5 Michael Stahl (allotropia) 2012-12-18 14:28:27 UTC
regression is a keyword...
Comment 6 Kohei Yoshida 2013-01-08 15:36:45 UTC
PIVOTTABLE has its own BSA keyword.
Comment 7 ktos.obcy+freedesktop 2013-01-29 10:50:02 UTC
Just tested with the latest 4.0RC2 and the bug is still present.
Comment 8 Joel Madero 2013-01-29 16:26:36 UTC
Version is the oldest version that we can confirm the problem, not the latest it's been tested on. 

We use comments to say "we tested it on X.X.X.X and it's still present
Comment 9 ktos.obcy+freedesktop 2013-02-07 15:40:47 UTC
OK, mea culpa.

As expected same is happening in 4.0.0 final.

I've been playing with it for a bit and it seems that the issue is related to the output of the function being /empty/ - i.e. double quotes (to have blank field - it seems that then Cals ignores source formatting and assumes the input/output to be plain numeric/text determining pivot table formatting) and also from the fact that (as it seems) formatting of the whole pivot table is taken from last row of the input table selection. I'm going to attach simple testcase sheet. In that testcase if you remove start/end time from last row and refresh pivot table then the issue manifest itself.

for the moment I've "fixed" my initial sheet by replacing conditional function:
=IF(F278-E278>0;F278-E278;"")
with simple /0/ (also, using =IF(F278-E278>0;F278-E278;0) would work) as then times are displayed on in whole pivot table.
Comment 10 ktos.obcy+freedesktop 2013-02-07 15:42:44 UTC
Created attachment 74353 [details]
formatting of the pivot table is determined by the last row content

removing start/end from last row thus causing IF function to output blank (i.e. double quote - "") causes pivot table to assume that the whole column is text forcing different formatting
Comment 11 Kohei Yoshida 2013-03-07 15:34:52 UTC
(In reply to comment #10)
> Created attachment 74353 [details]
> formatting of the pivot table is determined by the last row content
> 
> removing start/end from last row thus causing IF function to output blank
> (i.e. double quote - "") causes pivot table to assume that the whole column
> is text forcing different formatting

Very good observation.  This narrows it down significantly.
Comment 12 Kohei Yoshida 2013-03-07 15:38:22 UTC
The difficulty is how to determine the "representative" number format for a column when the column contains a mixture of number formats.  Counting solves it of course, but that would affect performance a bit if done incorrectly.
Comment 13 Kohei Yoshida 2013-03-07 16:19:09 UTC
I think the old code took the number format of the first non-empty cell, while the new code takes the last number format.  Either way is not perfect.
Comment 14 ktos.obcy+freedesktop 2013-03-07 16:27:41 UTC
OK, I think that taking first item format makes more sense than last one. Second think - can it take format set for the given cell - first (or last, tho not too fond of that) and not the format derived by Calc? For example I format whole column as "time" ([hh]:mm - see examples) but for some cells I don't want to display anything thus I use =IF(cond;val;"") which ten puts empty string (i.e. >> "" << ) into cell and for no apparent reason Calc suddenly treats the cell as string/text and not as 'time' as set in formatting.
Comment 15 Commit Notification 2013-03-07 17:03:14 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "master":

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

fdo#54651: Only pick non-default number format for pivot field.



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 16 Kohei Yoshida 2013-03-07 17:06:18 UTC
(In reply to comment #14)
> OK, I think that taking first item format makes more sense than last one.
> Second think - can it take format set for the given cell - first (or last,
> tho not too fond of that) and not the format derived by Calc? For example I
> format whole column as "time" ([hh]:mm - see examples) but for some cells I
> don't want to display anything thus I use =IF(cond;val;"") which ten puts
> empty string (i.e. >> "" << ) into cell and for no apparent reason Calc
> suddenly treats the cell as string/text and not as 'time' as set in
> formatting.

The fix I just pushed should take care of that.  If not, since that's a separate issue, please file a new bug.

FYI I've proposed a backport of my fix into the 4.0 branch as well.
Comment 17 Commit Notification 2013-03-07 17:50:01 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

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

fdo#54651: Only pick non-default number format for pivot field.


It will be available in LibreOffice 4.0.2.

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 18 Kohei Yoshida 2013-03-07 18:00:59 UTC
The fix is in the 4.0 branch.  I'll call it fixed.
Comment 19 Björn Michaelsen 2013-04-15 15:05:08 UTC
bibisect shows 32b61ed8931acd97e488bc73486244c385a3a974 to be bad, which was on 2012-03-08 long before 3.6 branchoff, thus not a 3.6.0->3.6.x minor release regression and the bug was at least present as early as 3.6.beta.
Comment 20 Commit Notification 2014-02-23 05:43:26 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

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

OOXML export validation fixes, related fdo#54651



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.