Item1406: SpreadSheetPlugin and EditTablePlugin cannot coexist

Example 1

First we define sumform1 %CALC{"$SET(sumform1, $NOEXEC($ROUND($SUM(R2:C2..R$ROW(-1):C2),0))) "}%

Here is a table with EDITTABLE

Item	Cost
Total	30.0
Motor	10
Wheels	10
Magnet	10

%CALC{"$ROUND($SUM(R2:C2..R$ROW(-1):C2),0)"}% gives 30

%CALC{"$EXEC($GET(sumform1))"}% gives 30

Conclusion

We can set a variable in CALC and use it inside the table

But we cannot run a CALC outside the table that operates on rows and columns.

Example 2

First we define sumform2 %CALC{"$SET(sumform2, $NOEXEC($ROUND($SUM(R2:C2..R$ROW(-1):C2),0))) "}%

Here is a table without EDITTABLE

Item	Cost
Total	60.0
Motor	20
Wheels	20
Magnet	20

%CALC{"$ROUND($SUM(R2:C2..R$ROW(-1):C2),0)"}% gives 60

%CALC{"$EXEC($GET(sumform2))"}% gives 60

Conclusion

We can set a variable in CALC and use it inside the table

And we can run a CALC outside the table that operates on rows and columns.

Example 3

Next example is again an EDITTABLE

Item	Cost
Total	90
Motor	30
Wheels	30
Magnet	30

%CALC{"$ROUND($SUM(R2:C2..R$ROW(-1):C2),0)"}% gives 90

Conclusion

The 60 comes from Example 2.

So SpreadSheetPlugin simply cannot see the tables as table.

Now why is that?

EditTablePlugin must be doing something to the table in the beforeCommonTagsHandler that makes it invisible to SpreadSheetPlugin which operates in the commonTagsHandler. Ie after ETP has done its job.

Example 4

First we define sumform4 %CALC{"$SET(sumform4, $NOEXEC($ROUND($SUM(R2:C2..R$ROW(-1):C2),0))) "}%

Here is a table with EDITTABLE

This time we try to set a variable inside the table where we know the SSP can see the rows.

Item	Cost
Total	120.0
Motor	40
Wheels	40
Magnet	40

%CALC{"$GET(sum4)"}% gives 120.0

Once we are outside the table I still cannot get to the variable that has been set inside. This is highly odd.

-- KennethLavrsen - 26 Apr 2009 has refactored the original bug report submitted by -- GuentherFischer - 02 Apr 2009

Oh - as I see now - this is not at foswiki.org but on my side ... I will look fpr an update of SpreadSheetPlugin ...

-- GuentherFischer - 02 Apr 2009

I found a newer SpreadSheetPlugin - this doesn't change the result - we see the result value as 0.

-- GuentherFischer - 02 Apr 2009

So - now I tried it on http://trunk.foswiki.org/Sandbox/SpreadSheetTest and you can see the same error as on my side ...

-- GuentherFischer - 02 Apr 2009

The same error with foswiki-1-0-5 - http://trunk.foswiki.org/Sandbox/SpreadSheetTest has the error too. foswiki.org not, but this is allways 1.0.0 -> WHY ???

-- GuentherFischer - 26 Apr 2009

I think the issue is not SpreadSheetPlugin but the changes done in EditTablePlugin.

To fix some of the things in EditTablePlugin - bugs that are also very important - we are doing more in the beforeCommonTagsHandler and before the SpreadSheetPlugins gets a chance to do it work.

Each time we touch EditTablePlugin, it takes 3-4 weeks to get it stable again so we have to be careful. And we had a deadline to meet on the 1.0.5 release.

-- KennethLavrsen - 26 Apr 2009

It is often advantage to destill a problem down to its roots.

Your examples were hard to destill because they tried to do too much at the same sime.

I have completely refactored your report with 4 new examples that are easier for developers to work with.

The conclusions are

SSP cannot see tables if it is an EDITTABLE

And if SSP sets a variable inside a table, it cannot be seen outside.

My feeling is that it is the Foswiki::Func::expandCommonVariables run in beforeCommonTagsHandler that makes the variables set inside invisible to the outside.

But I still wonder why the SSP plugin cannot see the tables once it kicks in.

If we undo the improvements in ETP we end up with the old problem again - that macros inside EDITTABLE tables are expanded and saved as their value.

This one will be tough to solve. Very tough.

Raising to urgent because this bug will break 100s of applications.

The only workaround I can give to people at the moment is not to use EDITTABLE tags when you need to calculate with values outside a table.

-- KennethLavrsen - 26 Apr 2009

I have an interesting observation.

If I make this little modification to EditTablePlugin.

Index: EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin/Core.pm
===================================================================
--- EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin/Core.pm (revision 3706)
+++ EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin/Core.pm (working copy)
@@ -568,8 +568,8 @@
         }

         # render variables (only in view mode)
-        $resultText = Foswiki::Func::expandCommonVariables($resultText)
-          if ( !$doEdit && ( $mode & $MODE->{READ} ) );
+#        $resultText = Foswiki::Func::expandCommonVariables($resultText)
+#          if ( !$doEdit && ( $mode & $MODE->{READ} ) );
         $topicText =~ s/<!--%EDITTABLESTUB\{$tableNr\}%-->/$resultText/;

         # END PUT PROCESSED TABLE BACK IN TEXT

then the CALCs are no longer rendered.

BUT

If I then also changes the plugin running order in configure.

$Foswiki::cfg{PluginsOrder} = 'TWikiCompatibilityPlugin,EditTablePlugin,SpreadSheetPlugin,CommentPlugin,ActionTrackerPlugin';

Ie. I make sure EditTablePlugin runs before SpreadSheetPlugin, then everything seems to work as it should.

Now this is with 10 minutes testing and I doubt it is THAT simple to resolve.

I will naturally need to test a lot more. This could break other things.

-- KennethLavrsen - 26 Apr 2009

That was indeed too easy a fix. I can see failures when I run my manual test cases. But I am on to something right here.

The error I see is that CALCs are causing trouble when in a label tag. When in a normal edit field all is fine.

-- KennethLavrsen - 26 Apr 2009

I am working on a fix

I have something I think will work well

Need to test more

-- KennethLavrsen - 27 Apr 2009

I have now worked on this for 4 days.

And I am now at the point where I know why it fails, and what to do about it.

But it is too large a code job for me to do alone. I need help.

Let me explain exactly what happens.

The EditTablePlugin is made so it protects all Macros inside tables from being expanded. And to do this it operates in both beforeCommonTagsHandler and CommonTagsHandler.

When you just view a topic with one or more EDITTABLE tags this happens

• During beforeCommonTagsHandler the topic is parced for tables with an EDITTABLE tag. If such table is present it gets analysed and all cells neatly placed in a matrix. Finally the table is complete removed together with EDITTABLE and TABLE tags
• When we get to commonTagsHandler the SpreadSheetPlugin runs before all other plugins and does it job. CALC inside EDITTABLE tables are not processed because they are hidden away in a matrix variable.
• Eventually EditTablePlugin runs its commonTagsHandler and restores all the tables again. If you are editing a table, the table being edited is displayed in edit mode.
• One of the last things EditTablePlugin does each time it puts back a table is to run the table through Foswiki::Func::expandCommonVariables which will re-invoke the SpreadSheetPlugin to calculate the CALCs.

The problem is that the CALCs outside tables are being expanded BEFORE the CALCs inside tables and without visibility to tables with an EDITTABLE tag. So all CALCs inside tables work fine. Those outside that use the tables or variables set by CALC in a table cell will typically fail or show 0.

I have tried many models including taking SpreadSheetPlugin out of the plugins order so it runs after EditTablePlugin.

But then you have many other unwanted side effects. I have been able to hack most of them out but not all and each time I remove one bumb on the carpet by beating on it another comes up.

What I have learned and concluded is

• It is not a good idea to take out all EDITTABLE tables in beforeCommonTagsHandler
• It is a good idea to use the beforeCommonTagsHandler to protect the table being edited. It is only the table being edited that is the problem anyway
• Instead of the current scheme I believe that
  • beforeCommonTagsHandler should parse the tables but not take them out unless the table is being edited.
  • commonTagsHandler should parse the topic and
    • put the edited table back exactly the way it does it now. It works very well.
    • add the edittable buttons to the other tables.

I cannot see any small easy fix that cures this problem.

And before anyone says EditRowPlugin. I looked at it again. It does some things right. And many things wrong. I will have to spend one day testing it in all corners to get all the issues listed as bug reports. Examples of bugs are coexistance problems with TablePlugin, missing features like the moving of rows more than one position at a time and plain errors like handling of header rows. And some usability issues also. There is many months of work getting that stable. We still need to get EditTablePlugin fixed and I think we have all the right code in the plugin now. We just need to apply it a little differently in the sequence of execusion.

It is probably mainly Arthur that will feel motivated to help me with this, but other can help with ideas also. I hope we can chat about it in #foswiki in near future. I do not want to let go of this one but I must admit that I now need help.

-- KennethLavrsen - 01 May 2009

I now see the reports comming in internally after I upgraded our site to 1.0.5.

Many people are hit by this bug. Even my own project management plan cannot calculate anymore because I have a topic with an edit table which is included in a project management plan. The topic works. The project management plan does not unless I remove the EDITTABLE tag.

This is a major pain. I have department managers calling now saying their weekly reports have stopped working. Again because they use CALC inside EDITTABLE tables and then include the topics.

-- KennethLavrsen - 05 May 2009

Thanks for your research, Kenneth. We will have to find a way around the special treatment of TML inside edit tables. Except for your analysis, did you make a start refactoring the code?

-- ArthurClemens - 05 May 2009

No refactor yet.

I am prepared to spend a lot of time on this. But before I start coding I would like to agree with your Arthur what the best method is. And I will for sure need help along the way.

I outlined a proposal above. Does it make sense? Did I miss something?

-- KennethLavrsen - 05 May 2009

Is it important that CALC values are correctly calculated on the page when editing? If not important, it looks I have a quick fix (altough it will destroy the earlier fix for Tasks.Item5146, having search results separate from the edit table).

-- ArthurClemens - 08 May 2009

There are two answers in this.

• I cannot see it is important that CALC values are correct when editing a table. And if you edit values in current table the CALCs will be wrong anyway. And I assume the more basic CALCs will still work.
• I have already myself started using the fix you did in 5146. I have some schedules where I show a table that consists of the local edit table and a search result. I could never do this before because I would end up "importing" the search result into the edited table. What will it destroy? And can we perhaps work around it? I would not mind that the search result table would be displayed wrong or displayed below as a separate table with white space between the edited table and the search result.

With repect to CALC inside the table being edited. I have been working on different models including the one we agreed to try with static fields. The best result in practical use is to simply replace the CALC by the text string CALC while editing. Maybe if it was styled gray it would be better but the plain line

$text =~ s/$PATTERN_SPREADSHEETPLUGIN_CALC/CALC/go;

is by far the best I have tried. It works in large tables with many small numbers because the short 4 characters do not make columns wide and you have no doubt what you can edit and what is a label.

I would like to see your quick fix. Can you attach a diff perhaps?

-- KennethLavrsen - 09 May 2009

Yes, this is the change (to EditTablePlugin.pm only)

• EditTablePlugin.pm.diff: Diff 20090508

-- ArthurClemens - 09 May 2009

To prevent the edit table from adding search results to the table in edit mode, first the etrows param needs to be recoded. This is now written as hidden field in the view mode of the table, and when editing the code let the table have this number of rows. I think this is not so useful (we are better of with a param that lets us insert of remove a number of rows at a certain position), but this code is in a lot of places.

-- ArthurClemens - 09 May 2009

I tested a little bit tonight but will continue Sunday.

With a table that has a search that appends its search result as additional table rows things get really messed up when editing.

When you edit the tabled gets as many empty rows appended as there are rows in the search. And these get saved when you save.

So we need to address this somehow. You seem to have an idea of what it takes Arthur.

-- KennethLavrsen - 10 May 2009

I have started working at this.

-- ArthurClemens - 30 May 2009

I am getting progress.

-- ArthurClemens - 05 Jun 2009

Perl code is now working. I need to update the javascript and then the unit tests.

-- ArthurClemens - 07 Jun 2009

Coding done, as well as javascript and unit test updates. Please test, test, test.

-- ArthurClemens - 14 Jun 2009