Issue 77648

Created attachment 45290 [details]
Bug document

Created attachment 45359 [details]
The patch file

I have fixed this issue, please have a look at the patch file.

Mmh, even your patch would solve this issue as well as issue 77649, I have some
concerns about it.
It would change the behavior of the function SwTableBox::IsValidNumTxtNd(..).
At the moment this function works for simple cases (only paragraphs, no
sections, no table in table) in the following way: It returns "false" if the
cell contains more than one paragraph. With your patch, even two or more
paragraphs would be accepted as "ValidNumTxt".
I would prefer a fix which does not change the behavior for these cases but
solve only the buggy ones.
Please have a look at the code (SwTableBox::IsValidNumTxtNd(..))
Instead of removing the check if the next node is an end node this check has to
be enhanced. A loop is required. Behind the text node are end nodes allowed if
it belongs to section start nodes. The first end node which does not belong to a
section start node will be the end of a cell. Then it has to be checked if this
cell is identical with the current table box to avoid a change in a table in table.
Oops, my idea would fix issue 77649, but not this one :-(
To fix this one, the Undo object which calls the function above has to be
change, I assume.

Created attachment 45454 [details]
The patch file

Hi ama, according to your suggestion, I have changed the patch. To fix this 
issue I didn't touch the function SwTableBox::IsValidNumTxtNd(..), I add some 
code to the areas (call this function) to get the node (number format) postion 
in the cell(contains more than one paragraph).
Please have a look.

Created attachment 45455 [details]
The patch file

The latest patch(2007/05/28 06:56:00 +0000: patch.diff) contains all the codes 
for this issue.Please have a look.

The latest patch(2007/05/28 06:56:00 +0000: patch.diff) contains all the codes 
for this issue.Please have a look.

The latest patch(2007/05/28 06:56:00 +0000: patch.diff) contains all the codes 
for this issue.Please have a look.

Add dependency to meta issue

add myself

Sorry, if I've been unclear. I wrote this issue because the "Undo" does not work
correctly. The action itself (changing number format from 'text' to 'date')
works as expected.
What I mean:
If you change the number format of a cell, two things should happen:
1. the number format of the cell changes (regardless of the content of the cell)
2. if the cell contains one single paragraph, this text will be reinterpreted
for the new number format, but if there is more than one paragraph in the cell,
the text should _not_ be reinterpreted.

Undo should change back the text if step 2. has been performed and of course,
step 1.
The problem is that Undo actually works only if step 1. _and_ step 2. has been
performed. If the condition of step 2. has not been fulfilled, neither step 2.
(which is okay) nor step 1. (that's the bug) will be undone.

Your patch would change the behavior in a way that step 1. and step 2. will be
performed regardless of the content of the cell. This would of course solve the
undo-problem but it would change the wanted behavior.
For this issue you should not change IsValidNumTxtNd(..) or their calling
functions in a way that step 2. will performed even there is more than one
paragraph. 
You should instead try to record the change of step 1 even when step 2. is
skipped. This does not work at the moment and is the reason for the failing of undo.

For issue 77649 we have to fix IsValidNumTxtNd(..) itself, I will provide some
comments into the other issue as well.

Created attachment 45503 [details]
the patch file

Created attachment 45504 [details]
the correct patch file

So the requirement is:
After change the cell's (has two or more paragraph) number format,
do undo will undo the operation before the change.
Not to make the cell can be changed.
Right?

Created attachment 45507 [details]
the patch file for doing undo to change the number format back to "text"

The latest patch is for the operation:
After change the cell's (has two or more paragraph) number format,
do undo will just change the number format back to "text".

This patch would not work if the two or more paragraphs are inside a section.
BTW: Every SwTxtNode is a SwCntntNode as well, it's not necessary to test
"IsTxtNode() && IsCntntNode()"

Created attachment 45545 [details]
the patch file

I think the patch has to cover all the situation (the cell contains complex 
paragraph and the function SwTableBox::IsValidNumTxtNd(..) return ULONG_MAX),
I think the latest patch could be fit to this issue.

Unfortunately I do not have the time to test your patch in real life, but I had
a look at the code. I think that it solves the situation I mentioned so far, but...
I could imagine more problems:
1. Please check the "Redo" functionality; I doubt it will work for cells with
more than one paragraph.
2. Please check the Undo if your cell contains "Hello world" with an attribute
like 'bold' attached to the second word ('world'), I fear this attribute will be
deleted by "Undo"?!

The solution:
The old code did two things if it found one paragraph, it changed the text and
it changed the cell format and recorded both changes.
You could separate this two items ( box format changes, text changes).
The box format changes should be done always, undoable and redoable. The text
change should only be done, if IsValidNumTxtNd() does not delivers ULONG_MAX. In
this case, all the text node related code has to be done in Ctor, Undo and Redo.

Created attachment 46125 [details]
the patch file

Created attachment 46126 [details]
the patch file

I have changed the patch file according to your suggestions. Please have a look 
at it.

Created attachment 46146 [details]
Modified patch

Fine, looks great! Thank you!
I have modified your patch slightly and will integrate it into CWS swqbf99.

Fixed in CWS swqbf99
untbl.cxx

Ready for QA

Verified in CWS swqbf99.

Checked fix in 680m222.