Setting it as enhancement so that it can be tracked better...

CC

(In reply to comment #1)
> Setting it as enhancement so that it can be tracked better...
I do not understand. A bug is not an enhancement imho.
pz1

Still unfixed in: LibreOffice 3.3.0 OOO330m19 (Build:5)tag libreoffice-3.3.0.3

Still not fixed in: LibreOffice 3.3.1 OOO330m19 (Build:5)tag libreoffice-3.3.1.1

Proposing it as an EasyHack.  Patches are welcome.

I'll give this to Cedric meanwhile.

Cedric, please check if this can be an easy hack.  If not, feel free to remove the tag.

for the code, look in 
 http://opengrok.libreoffice.org/xref/writer/sw/inc/tblafmt.hxx
and
 http://opengrok.libreoffice.org/xref/writer/sw/source/ui/table/tautofmt.cxx

For Easyhackers I am available as a tester!

Set back to major - loss of formatting shown as DEFECT in OOo issue tracker.

updated links from cor, comment 8 (after one git repository conversion) :
 http://opengrok.libreoffice.org/xref/core/sw/inc/tblafmt.hxx
and
 http://opengrok.libreoffice.org/xref/core/sw/source/ui/table/tautofmt.cxx

Thanks to those who looked into this problem. Apparently this is not going to be an easy hack. Maybe it is a misconception to look on a table in Writer as if it were a range of cells in a spreadsheet?

Investigating.

I've been investigating why cell spacing isn't saved/loaded correctly. Here's the story so far.

Summary:
========
The loss of individual spacing values happens on save to ~/.libreoffice/autotbl.fmt. For autotbl.fmt versions 3.1 and 4.0, the saving code writes the smallest spacing value to file, and that value is used for all 4 (left, top, right, and bottom). For versions 5.0+, the 4 spacing values are stored and loaded. However, we're currently using the 4.0 format for autoformat, so we lose data.

Details:
========
The format saving happens when you close the AutoFormat dialog.

* The entry point in the code is SwTableAutoFmt::Save (http://opengrok.libreoffice.org/xref/core/sw/source/core/doc/tblafmt.cxx#858).

* The loop in SwTableAutoFmt::Save calls SwBoxAutoFmt::Save (http://opengrok.libreoffice.org/xref/core/sw/source/core/doc/tblafmt.cxx#450).

* SwBoxAutoFmt::Save stores the various properties, one by one. The cell spacing is stored in SwBoxAutoFmt::aBox. The saving code for that is SvxBoxItem::Store (http://opengrok.libreoffice.org/xref/core/editeng/source/items/frmitems.cxx#2139). It's called with a format version of 4.0:
       aBox.Store( rStream, aBox.GetVersion(SOFFICE_FILEFORMAT_40) );

* SvxBoxItem::GetVersion reports an item version of 0 when given SOFFICE_FILEFORMAT_40 (would've reported BOX_4DISTS_VERSION for FILEFORMAT_50+).

* SvxBoxItem::Store checks the item version (0) against BOX_4DISTS_VERSION, and decides not to store the 4 cell spacing values.

Next Steps:
===========
The ideal solution is to move the saving code to use a newer format version. It seems that the existing loading code can handle the new version just fine. The version handling code is a bit messy, so I need to investigate it a bit more to understand if increasing the file version will have any repercussions.

I will also investigate other properties that were reported broken to see if the same solution would fix them.

Updating the version of autotbl.fmt only fixes spacing. Most other properties aren't saved at all, it seems. Detailed breakdown follows. I'm currently trying to figure out how much work saving these would be. If it's not feasible to save them all, I will at least try to save the most common/important ones (e.g. manual table and column formatting is probably low priority, but break behavior and borders are important).

Table: None of these are saved
    . Width
        - Value
        - Relative (CheckBox)
    . Spacing
        - Left, Right, Above, Below (4 values)
    . Alignment
        - Automatic, Left, From left, Right, Center, Manual
    . Properties
        - Text Direction: Superordinate, LTR, RTL

Text Flow: None of these are saved
    . Break
        - Page/Column
        - Before/After
        - (If Page) With Page Style:
            - Style (enum)
            - Page number
    . Allow table to split
        - Allow row to break
    . Keep with next para
    . Repeat heading
        - Number of rows
    . Text Direction: Superordinate, LTR, RTL
    . Vertical Alignment: Top, Centered, Bottom

Columns: None of these are saved
    . Adapt table width
    . Adjust columns proportionally
    . Column widths (N columns)

Borders: None of these are saved except spacing
    . Line arrangement
        - Border style
        - User-defined
    . Line
        - Style
        - Width
        - Color
    . Spacing to contents
        - Left, Right, Top, Bottom
        - Synchronize
    . Shadow style
        - Position
        - Distance
        - Color
    . Properties
        - Merge adjacent line styles

Background: These are saved
    . As
        - Color or Graphic
    . For
        - Cell, Row, or Table
    . Background Color

I -think- you're likely to find that these ::Save methods are obsolete, and should be removed anyway ;-) - I -think- that they're vestiges of the old binary file format - which should (in theory) live only in the binfilter/ nowadays.

I imagine that the ODF import/export for this is done in a far more complicated way via UNO interfaces, and the code is split out and most likely lives in the 'xmloff' module somewhere - though chunks of it are still in sw/source/filter/xml/

Does that help ? :-) ( I assume you're targetting ODF ? ).

Without code pointers, this didn't make a terribly easy hack IMHO ;-)

(In reply to comment #16)
> I -think- you're likely to find that these ::Save methods are obsolete, and should be removed anyway ;-) - I -think- that they're vestiges of the old binary
> file format - which should (in theory) live only in the binfilter/ nowadays.
> 
> I imagine that the ODF import/export for this is done in a far more complicated way via UNO interfaces, and the code is split out and most likely lives in the
> 'xmloff' module somewhere - though chunks of it are still in sw/source/filter/xml/
> 
> Does that help ? :-) ( I assume you're targetting ODF ? ).
> 
> Without code pointers, this didn't make a terribly easy hack IMHO ;-)

These Save methods are still invoked (I stepped through gdb), but they're not for the documents. The saved "Table AutoFormats" go into a file of their own, ~/.libreoffice/autotbl.fmt, and it's versioned separately. Judging from the version history for auto formats, people were just adding properties haphazardly or on demand. I'd like to fix it once and for all if possible.

I agree it's not an EasyHack, but I'm already in, and it seems within reach :)

Oh !

> These Save methods are still invoked (I stepped through gdb), but they're
> not for the documents. The saved "Table AutoFormats" go into a file of
> their own, ~/.libreoffice/autotbl.fmt, and it's versioned separately.
> Judging from the version history for auto formats, people were just
> adding properties haphazardly or on demand. I'd like to fix it once
> and for all if possible.

Silly me :-) you're doing a great job. Clearly it'd be nicer to use an XML format (if we could), then perhaps, later we could shove it into the ODT files if people want to embed them, but ... anyhow - well done for getting on top of it.

Progress update: I finally figured out how to get table-level properties to be saved in AutoFormat (I currently have "Text Flow->Break" working). It's a bit hackish in that it doesn't integrate with undo/redo, but it's a good start. I'm currently looking into enabling undo/redo, as well as adding the remaining properties.

Next update (and perhaps a preliminary patch) expected around Tuesday.

Update: Text flow options (from the "Text Flow" tab in table properties) work, and I'm working on the options from the "Borders" tab (currently only shadow and spacing work). I hit a snag with some table properties which delayed the fix for a while, but it's sorted out now, so things should go smoother.

Created attachment 60462 [details]
Proposed patch.

Added patch. This patch is mainly targeted at Writer autoformats, although some improvements apply to Calc.

There are some known issues with this patch that I'll probably open individual bugs for and address one by one, unless I get strong feedback in favor of fixing them with this.

I'll shoot an email to the dev mailing list later tonight or tomorrow, asking for review. It's highly likely there'll be more work on the patch before it's accepted, since it's a bit messy due to the sharing of autoformats between Calc and Writer.

I've also discovered a bunch of bugs while working on this, and I'll be reporting them in the next couple of days.

Great! Many thanks for all your efforts in this apparantly not_so _easy_hack. I am looking forward to testing it.

Review thread on mailing list: http://nabble.documentfoundation.org/PATCH-fdo-31005-Table-Autoformats-does-not-save-apply-all-properties-td3936451.html
(No replies at the time of this comment)

Muhammad Haggag committed a patch related to this issue.
It has been pushed to "master":

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

fdo#31005 Writer: Undo support for "Repeat Heading" in autoformat application

Muhammad Haggag committed a patch related to this issue.
It has been pushed to "master":

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

fdo#31005 Table Autoformats do not save/apply all properties

(In reply to comment #22)
> Great! Many thanks for all your efforts in this apparantly not_so _easy_hack. I
> am looking forward to testing it.

@pz1, have you tested this? can we mark this bug as fixed?

If you haven't tested yet, please grab a daily build from http://dev-builds.libreoffice.org/daily/ and post result here. Thanks!

I have tested it the day after it was committed in a 3.6 alpha for Windows. I concluded that it does now save one table autoformat properly. And I am 99% sure it could be marked as solved. I had one more rigorous test in mind however. I do have a big 1000+ page document where I use autoformats applied by a complex macro. 

Unfortunately the windows release has a problem with JRE not being recognised. That has been reported as bug 48946 with importance HIGH/BLOCKER. That has not yet been resolved to date. There also was a possibly related macro bug that has recently been solved. I have not yet had the time to test after that fix. I'll do so in one or two days

I got this morning's Alpha1 for Windows. I run on XP.

I still have a problem. Reproduce:
* Create table in new document and place cursor in table
* Do via menus -Table/add/
* Fill-in 'name' and OK and once more OK
I get screen indicating crash telling me:
"Due to an unexpected error, LOdev crashed. All the files you were working on will now be saved. The next time LOdev is launched, your files will be recovered automatically."

I do not remember this error appearing during testing in alpha0 releases back in april/may.

Thanks for testing, but is this crash related to your 1000+ page document testing?
If not, could you please file a new bug and post the bug number here?

The point is:
* Bug 48946 is fixed, and a build this morning (UTC, June 4) should include the fix.
* Could you please test with your 1000+ page document if the crash isn't related?

Thanks again!

(In reply to comment #29)
> Thanks for testing, but is this crash related to your 1000+ page document
> testing?
> If not, could you please file a new bug and post the bug number here?
> 
> The point is:
> * Bug 48946 is fixed, and a build this morning (UTC, June 4) should include the
> fix.
> * Could you please test with your 1000+ page document if the crash isn't
> related?
> 
> Thanks again!

If you re-read my post you will see that it crashes on an empty document in which I have just created an empty table.
Next thing I did is assign the default format of that table to a named autoformat.
There it crashes. I did reboot my machine twice, and it happened again and again.
Nothing to do with my 1000+ page document.
(The macros are working again which I tested successfully on the first chapter of 66 pages)

So no need for opening a new issue. The problem is in saving the autoformat itself!

Sorry for that.

Confirmed the crash on Windows XP
* official alpha 1
* tinderbox W2008R2@16-minimal_build, pull time 2012-05-28 04:23:11, git core: b5f066e64a49aa007e04dc57dc0bbd0857ca0e2f

No crash on
* official 3.5.3
* tinderbox Win-x86@6-fast, pull time 2012-04-24 22:15:22, git core: 52d90ce040234cb35fad03bdd12e201bfa3a8634

Could you please contribute more information about "this morning build" like I did here? Thanks again.

(In reply to comment #31)

> Could you please contribute more information about "this morning build" like I
> did here? Thanks again.

The about box says:
version 3.6.0alpha1+ (Build ID: 793f240)

(In reply to comment #28)
> I got this morning's Alpha1 for Windows. I run on XP.
> 
> I still have a problem. Reproduce:
> * Create table in new document and place cursor in table
> * Do via menus -Table/add/
> * Fill-in 'name' and OK and once more OK
> I get screen indicating crash telling me:
> "Due to an unexpected error, LOdev crashed. All the files you were working on
> will now be saved. The next time LOdev is launched, your files will be
> recovered automatically."
> 
> I do not remember this error appearing during testing in alpha0 releases back
> in april/may.

I guess this problem is related with bug https://bugs.freedesktop.org/show_bug.cgi?id=50896

The newly introduced bug 50896 may have been solved in the meantime, I can not yet test the result, because the last nighly build of the Windows version is from 8th of June. That is from before the bug was fixed I assume. (The poster of the fix did not properly report in that bug report, so I can not tell for sure when it really took place)

The fix was committed at 2012-06-11 13:19:37, so you should grab a build from the 12th or later to be sure :)

I tested in Version 3.6.0beta1 (Build ID: 1f1cdd8), which I found this afternoon. This version still crashes on applying an Autoformat to a table, so I assume this won't be fixed before beta2 if at all. So I give up checking to see if it is fixed, until I get notification from this page.

It works OK in Version 3.6.0.0.beta2 (Build ID: f010139)
As far as I am concerned, this case is closed.
Thanks, Pieter

this is fixed as far as i can remember.

Migrating Whiteboard tags to Keywords: (EasyHack)
[NinjaEdit]

Notes for unit test writers:

Nothing to revert here, you would be testing new functionality.

The only remaining Writer Autoformat property to test is Break. The others were added in e0bafa78e3ad0df397d78cd65ad19bd5b07dc5f2

For Calc the tests seem to be missing, see test/source/sheet/tableautoformat.cxx