Apache OpenOffice (AOO) Bugzilla – Full Text Issue Listing
|Summary:||Display of hidden cells|
|Status:||CLOSED FIXED||QA Contact:||issues@graphics <issues>|
|Priority:||P3||CC:||amy2008, issues, kyoshida, yoshimit|
|Version:||3.3.0 or older (OOo)||Keywords:||ms_interoperability, usability|
|Issue Type:||FEATURE||Latest Confirmation in:||---|
Description IngridvdM 2008-07-11 15:03:30 UTC
When Charts are created from Calc ranges with partly hidden cells the user should be able to choose whether the hidden values should or should not be displayed in the chart.
Comment 2 IngridvdM 2008-07-11 15:20:17 UTC
It seems that the behavior for this scenario has changed several times in the past and always there were users that were not happy with the current behavior so it has become necessary to offer an option to choose.
Comment 3 IngridvdM 2008-07-11 15:23:52 UTC
*** Issue 80626 has been marked as a duplicate of this issue. ***
Comment 4 IngridvdM 2008-07-11 15:30:57 UTC
*** Issue 52800 has been marked as a duplicate of this issue. ***
Comment 5 IngridvdM 2008-07-11 15:56:29 UTC
In the user interface I could imagine a simple check box 'Display values from hidden cells' or similar. Two possible places for the check box come to my mind: 1) On the 'Data Range' tab page - just below 'First column as label' maybe. 2) 'Options' tab page within the series properties dialog. Issue 65549 in CWS chart22 does introduce the vaguely related setting 'Plot missing values' exactly on the series options page. So are there any user opinions about where to place the check box?
Comment 6 IngridvdM 2008-07-11 16:20:00 UTC
@kohei, yes for the file format we would need only a simple boolean. I think it would be elegant to define the new attribute in parallel to the somehow related attribute 'chart:treat-empty-cells' - having it in the style at the plot area. Here is a first try for the proposal: ---- The chart:display-hidden-cells attribute specifies whether values from hidden cells are displayed within the chart or not. This attribute can be used within styles that are applied to a <chart:plot-area> element. The chart:treat-empty-cells attribute may be used with the following element: <style:chart-properties> <define name="style-chart-properties-attlist" combine="interleave"> <optional> <attribute name="chart:display-hidden-cells"> <ref name="boolean"/> </attribute> </optional> </define> ---- I am not sure about a default. In the past the behavior has changed again and again. Kohei, as you are on the TC list, would you mind to propose this new attribute? With luck we can have it for ODF1.2 maybe. That would be cool.
Comment 7 IngridvdM 2008-07-11 16:35:31 UTC
Oh there is one wrong 'treat-empty-cells' in the proposed proposal text. That should be replaced with 'display-hidden-cells', sorry.
Comment 8 kyoshida 2008-07-11 16:46:53 UTC
Yeah I can work on proposing that to the TC. It should be fairly simple since it's just one boolean, but you never know.... I'll keep you updated with the progress.
Comment 9 IngridvdM 2008-07-11 17:19:25 UTC
@kohei, cool thanks! Some hints for the implementation: There is already a property that is very similar implemented to what we need now. It is the check box 'show bars side by side' on the options page in the series properties dialog for bar and column charts. So it can serve as example. In the UNO API we need a new property at the diagram maybe 'DisplayHiddenCells'. The stable ::com::sun::star::chart API is implemented in chart2/source/controller/chartapiwrapper. Add the new property to the DiagramWrapper class (good example is 'GroupBarsPerAxis'). The inner chart model uses not the old stable API but a new ::com::sun::star::chart2 API instead. There we need to add the same new property at the 'inner' diagram class ( look at chart2/source/model/main/Diagram.cxx). In xmloff we need a new token 'display-hidden-cells'. And the new token must be mapped to the new UNO property. Grep for XML_GROUP_BARS_PER_AXIS in xmloff for an example. Thus the new property will be written from and to the file. Now the new property must be transported to the view for display purposes. Search for 'GroupBarsPerAxis' within chart2/source/view/ChartView.cxx to have an example. The view does set the property from the diagram model to a view class 'VDataSeries'. We will need a new method SetDisplayHiddenCells(). VDataSeries is the container for the differenet VDataSequence where we finally need the property (you found the boolean bDisplayHiddenCells there already :-) ). Finally the dialog related things are implemented in chart2/ source/controller/dialogs and chart2/ source/controller/itemsetwrapper. All the ItemConverters within itemsetwrapper serve the purpose to transport the UNO API properties from the model object into SfxItemSets which is a typical input format for several dialogs and tab pages. And vice versa of course. I hope the hints are useful for you. Have fun!
Comment 10 kyoshida 2008-09-29 17:56:33 UTC
Ok. The ODF TC seems to be in favor of the change we have proposed. There are some minor suggestions for change, but none are blockers.  http://wiki.oasis-open.org/office/New_Chart_Option_on_Hidden_Cells
Comment 11 IngridvdM 2008-10-01 10:13:37 UTC
*** Issue 28932 has been marked as a duplicate of this issue. ***
Comment 12 kyoshida 2008-10-01 22:42:46 UTC
Comment 13 kyoshida 2008-10-07 15:19:49 UTC
I've pretty much finished all tasks except for the ODF import/export & check box in the UI. I'll work on those once we get approval on the ODF spec change from the TC.
Comment 14 kyoshida 2008-10-07 16:07:11 UTC
As for the location of the check box (sorry I didn't see that you asked for my opinion earlier), I would like the check box to stay close to the check box for the "plot missing values" option. These two options IMO should be placed side-by-side if possible.
Comment 15 IngridvdM 2008-10-07 16:32:12 UTC
Yes, that sounds like a good idea. The affected tab page class is SchOptionTabPage in files tp_SeriesToAxis.src -.cxx -hxx. The "plot missing values" option was introduced there in the 3.1 code line starting with dev300m29.
Comment 16 kyoshida 2008-10-31 14:16:20 UTC
BTW the core work is already done & ODF import/export completed. I will start the UI work once I get started on upstreaming the code (since the 3.0 codeline that I currently work on in ooo-build doesn't have the treat empty cells addition).
Comment 17 kyoshida 2008-11-03 20:58:46 UTC
just tried to create a cws for this. But 'cws create DEV300 charthiddencells' created a new svn branch but failed to connect to the EIS. I have manually created an entry for that branch via EIS web interface, but it's still incomplete. So, at the moment I have an out-of-sync cws branch that needs to be repaired by someone.... :-(
Comment 19 kyoshida 2008-11-26 06:06:15 UTC
Created attachment 58306 [details] new tab page for plotting options
Comment 20 kyoshida 2008-11-26 06:08:55 UTC
@iha: I've re-designed the Options tab page and attached the new layout. What do you think of this layout? I had to rename the fixed line text from 'Plot missing values' to 'Plot options' so that I could squeeze the include-hidden-cells check box there. I've gone through a couple of different layouts, but I like this one best. Let me know what you think.
Comment 21 kyoshida 2008-11-26 07:13:00 UTC
Created attachment 58307 [details] options tab page for pie chart
Comment 22 IngridvdM 2008-11-26 14:26:01 UTC
@kohei: Thanks! I think the layout could do. Maybe I would choose slightly different strings: 'Plot missing values' instead of 'Missing values' and 'Plot values from hidden cells' instead of 'Include hidden cells', but I am not a native speaker. I'll attach an according screen shot. Here is the link to the specification from the 'Missing value' feature: http://specs.openoffice.org/chart/MissingValues.odt I think it would be suitable to simply extend this specification to include the new check box. The spec needs to be changed anyhow now. Have a great day!
Comment 23 IngridvdM 2008-11-26 14:27:32 UTC
Created attachment 58323 [details] screenshot - different wording
Comment 24 kyoshida 2008-12-01 16:56:53 UTC
@iha: thanks! I think your wording sounds better. I'll make those changes & the spec (hopefully I still remember how to access it...) P.S. I just got back from our long holidays.
Comment 25 IngridvdM 2008-12-01 17:05:12 UTC
@kohei, the spec is located in module specs/www/chart/MissingValues.odt. It should be accessible via cvs still. P.S.: I hope you had nice holidays! I will start my longer vacation the day after tomorrow :-)
Comment 26 kyoshida 2008-12-01 19:06:41 UTC
Ok. Just updated the spec. @iha: do you think I can assign tk as the QA rep for koheichart01?
Comment 27 kyoshida 2008-12-01 21:09:51 UTC
This is now implemented in koheichart01 cws.
Comment 28 IngridvdM 2008-12-02 14:41:33 UTC
@kohei, > do you think I can assign tk as the QA rep for koheichart01? Do you mean Thomas Klarhöfer (kla)? Yes, you can. But he is already completely busy for 3.1 feature freeze. So if target 3.2 is ok for you then set Thomas as QA rep!
Comment 29 niklas.nebel 2008-12-04 10:18:43 UTC
Why are formulas set dirty when the chart starts listening (markRangeDirty)? That looks like a strange hack.
Comment 30 kyoshida 2008-12-04 12:54:58 UTC
@nn: that's not a hack. Under some circumstances value changes do not trigger formula broadcasters to send out value changed broadcast to their listeners, especially when the source ranges are outside the visible area and consist of formula cells. Marking them dirty is the only way to ensure that the broadcast occurs on value change.
Comment 31 niklas.nebel 2008-12-04 19:26:45 UTC
What are these circumstances? If formula broadcasts don't work, other areas beside charts are affected, and that should be fixed. If it's just about the charts, then the fix should be limited to charts, and not mess around with formulas. This just doesn't seem right.
Comment 32 kyoshida 2008-12-04 19:39:08 UTC
Setting formulas dirty is hardly "messing around with formulas" since no tokens are modified. Here are the circumstances. When source ranges are formula cells outside the visible area, that listens on value cells whose values get changed by the API. When the values of the listened cells change via API, those formula cells are not re-calculated until they become visible, which causes the chart to not get updated. Marking them dirty once when the chart gets activated solves this issue.
Comment 33 kyoshida 2008-12-04 19:50:55 UTC
BTW delaying re-calculation until they become visible is "correct" in its own right. That only becomes problem when an object such as chart depends on their values being up-to-date even when they are not visible. So, to me marking them dirty to force re-calculation on the next value change upon chart creation seems logical to me.
Comment 34 niklas.nebel 2008-12-08 18:34:43 UTC
Formula cells do of course re-broadcast changes even if they are not visible (there might be visible cells depending on them). A formula cell does not propagate the broadcast if it was already dirty, on the assumption that if the previous value wasn't used, all formulas that depend on the value are also still dirty from the previous broadcast. If you run into this because the cell values aren't read, what you need is the opposite of setting them dirty. Calculation (and with it, clearing the dirty flag) can be forced by reading the result value. ScFormulaCell::SetDirty does more than setting the flag, but you're relying on quite a chain of effects here, which seems too fragile.
Comment 35 kyoshida 2008-12-18 14:59:10 UTC
Ok. I've removed the controversial markRangeDirty method for now. That means that my test case that the method fixes will be back to a broken state, but at least it won't mess up the existing formula handling because of this code change. I will look into that formula issue separately. Since it's hard to reproduce it in a small test case document, and the only reproducing document I have is very large & impossible to debug with it, I'd like to tackle is as a separate task.
Comment 36 kyoshida 2008-12-18 21:49:56 UTC
Jon (Pryor) did a QA on this, and found a bug with the tool tips. The tool tips in a chart act as if all data points are shown. I need to look into this to fix it.
Comment 37 kyoshida 2008-12-24 15:12:08 UTC
Just fixed the tooltip issue. The tooltips inside chart objects now display proper data point values even when some of them are hidden. This is once again fixed in koheichart01 cws.
Comment 38 kyoshida 2009-01-05 20:42:27 UTC
Reopening it as we uncovered another regression. It is actually filed in Issue 97738, so the detail of the regression is described there.
Comment 39 kyoshida 2009-01-05 20:43:58 UTC
In short, the x-axis (category axis) fails to display its values if they are numeric, which also includes dates. I'm looking in this here: https://bugzilla.novell.com/show_bug.cgi?id=463199
Comment 40 kyoshida 2009-01-05 22:52:37 UTC
just fixed the x-axis issue.
Comment 41 kyoshida 2009-01-14 18:04:48 UTC
Re-opening this again. The tooltip issue is not entirely fixed. The value is displayed correctly, but the category name isn't. I need to fix this.
Comment 42 kyoshida 2009-01-14 18:50:16 UTC
Luckily it was not so hard to fix. Fixed again.
Comment 43 IngridvdM 2009-02-13 14:08:30 UTC
@kohei, I found that the current version does break charts with categories from writer tables. The properties 'IsHidden' and 'HiddenValues' are optional and not supported by writer so far. Load the attached document ChartFromWriterTable.odt, double click the chart and you will see how the diagram vanishes, what it should not.
Comment 44 IngridvdM 2009-02-13 14:09:26 UTC
Created attachment 60139 [details] example chart with data from a writer table
Comment 45 IngridvdM 2009-02-13 14:09:59 UTC
Another problem is with charts that do not have a range for categories. For those charts there should be automatically generated some strings, but they are missing now. Load the attached document ChartWithoutCategoryRange.ods and double click on the chart. You will see how the categories vanish what they should not.
Comment 46 IngridvdM 2009-02-13 14:10:40 UTC
Created attachment 60140 [details] chart without a range for categories
Comment 47 IngridvdM 2009-02-13 14:11:29 UTC
@kohei, I have fixed both problems already locally and could commit the changes directly to the CWS. Would that be OK for you?
Comment 48 kyoshida 2009-02-13 14:21:59 UTC
@iha: yup, that's fine. Please commit away. :-) BTW, both problems sound familiar. Those were something we had encountered in ooo-build and I thought I had already fixed that in koheichart01 as well... :-/ Anyway, thanks for the fix.
Comment 49 kyoshida 2009-02-13 14:37:13 UTC
@iha: BTW I'm going to resync that cws once koheiformula02 gets integrated. koheiformula02 modified chart2uno.[hc]xx quite a lot, so I have no doubt that its integration will clash with the change in koheichart01.
Comment 50 IngridvdM 2009-02-13 14:43:48 UTC
I have commited fixes for both above described problems to CWS koheichart01. @kohei, are there more bugs in CWS koheichart01 that are already known? I am asking because I am willing to have a deeper look into the chart changes on this CWS and have the time to do so at the moment. But if it is an outdated version I'd better wait until you have brought the known fixes into it maybe. What do you think does it make sense to look at further at the moment? I have seen for example that the legend entries for 'hidden' series do not hide.
Comment 51 kyoshida 2009-02-13 14:57:33 UTC
@iha: no, it's not outdated. I have made every effort to sync this cws with ooo-build's. Those fixes that I mentioned were already in koheichart01 (as I just checked) but I guess it didn't cover the problems you just found. I was not aware of the legend entry breakage, so most likely it's still broken. And I'm not aware of any other bugs that may be in koheichart01. So, please go ahead and fix any problems you find in that cws. :-) Thanks!
Comment 52 IngridvdM 2009-02-13 15:02:14 UTC
Ok fine! :-) So just drop me a note before you resync thus I stop commiting then ;-)
Comment 53 IngridvdM 2009-02-13 16:09:19 UTC
I found and fixed a layout problem with the new controls in case of stock chart.
Comment 54 IngridvdM 2009-02-17 10:16:53 UTC
Fixed another problem: Load the attached document 'HiddenSeries.ods' and double click on the two charts. There should not be a space between the two series. And there should not be a legend entry for Column E because it is hidden and should not be plotted.
Comment 55 IngridvdM 2009-02-17 10:18:05 UTC
Created attachment 60240 [details] example with wrong space and wrong legend entry
Comment 56 kyoshida 2009-02-17 16:27:31 UTC
Yup, verified both problems (and the fixes). Thanks!
Comment 57 IngridvdM 2009-02-17 17:13:47 UTC
Fixed another problem within the range highlighting: Load the attached file 'HiddenSeries.ods'. Within the column chart select the second data point of the first(blue) series. What should happen is that cell D9 gets highlighted. I will look for more tomorrow. This feature spreads quite ugly over the chart code as it affects potentially all places where ranges are used.
Comment 58 IngridvdM 2009-02-18 19:06:43 UTC
The feature 'Plot missing values' is broken by this CWS. The data comes already wrong from the calc. m_aDataArray in ScChart2DataSequence contains wrong Items. mfValue is zero where it should be NAN and mbIsValue is true where it should be false. I'll attach a document 'MissingValue.ods' to illustrate the problem. @kohei, as this is calc code could you please have a look what is going wrong there? Thanks!
Comment 59 IngridvdM 2009-02-18 19:10:02 UTC
Created attachment 60289 [details] double click the chart to see the problem with missing values
Comment 60 IngridvdM 2009-02-18 19:20:31 UTC
I fixed another problem with that feature. The radio button 'Leave Gap' could not be clicked anymore. When arranging controls side by side one must be careful that the controls do not overlap otherwise the underlying control is not reachable anymore. Those controls are sized and placed dynamically now.
Comment 61 kyoshida 2009-02-18 19:45:51 UTC
hmm... That UI was working before though. I remember I had to work on that to dynamically relocate the controls...
Comment 62 kyoshida 2009-02-19 05:53:28 UTC
Ok. The missing value treatment issue is now fixed. The fix was very easy; just changing the default values of Item's members to non-value was enough to fix it.
Comment 63 IngridvdM 2009-02-19 10:24:49 UTC
@kohei, thanks for the fix. I thought also that changing the constructor defaults might be sufficient. I found that the same error is contained in CWS koheiformula02. I am going to transport the fix therein. Regarding the UI - it was not working before my first checkins to this CWS. I have that version installed still. The relocation was dynamically in y direction already that is right but not in size and x direction. Well it is a pain to not have a layout engine - sigh. A little help here comes with the VCL debug options. You can activate them with Ctrl-Shift-Alt-D. Check the check box 'Dialog' and the current size of the controls will be displayed as green rectangle if you open a new dialog.
Comment 64 IngridvdM 2009-02-27 17:55:13 UTC
I found and fixed some more problems: When copying a chart with hidden and not plotted data and pasting it into writer or impress the hidden data is not hidden anymore. The nasty thing here is that the error is not visible immediately - at least in impress. If you double click the chart or start your presentation you will see that your copy looks different than the original. Another problem was with the trendline equation that is displayed in the status bar. Open the attached document TrendlineEquation.ods and select the trend line. Look at the text in the status bar. The formula should read f(x)=x and not f(x)=0.6571x+0.1905.
Comment 65 IngridvdM 2009-02-27 17:57:07 UTC
Created attachment 60555 [details] example for status bar text of selected trenline
Comment 66 IngridvdM 2009-02-27 18:03:48 UTC
It makes no sense to fix another place and another place and another place where values from ranges are used, so I redesigned the interface for this feature. The chart now controls the data provider and the data sequences which got a new property 'IncludeHiddenCells'. If that property is set to false the hidden values are not propagated to the chart.
Comment 67 kyoshida 2009-02-27 22:22:52 UTC
@iha: Yup, that design change makes sense. Thanks for keeping me in the loop.
Comment 68 IngridvdM 2009-03-04 16:41:18 UTC
Some more changes: I changed the default. Now newly created charts do not display values from hidden cells. This is what is expected for user scenarios with charts from filtered data (see issue 81209). The new check box now is only visible in case the data provider does support this feature. So currently the check box is only visible if the chart does retrieve its data from a spreadsheet. I updated the specification accordingly and send it to the other i-Team member for review: http://specs.openoffice.org/chart/MissingValues.odt There was also a bug with undo/redo after I changed the interface design. And finally I removed some code that was originally introduced for this feature but finally not used. I hope thats it now - but waiting for review feedback.
Comment 69 IngridvdM 2009-03-16 11:55:09 UTC
Feedback from i-Team received: The text from the new check box should better be 'Include values from hidden cells' instead of 'Plot values from hidden cells' as the latter text might be misinterpreted that 'only' values from hidden cells are plotted. ->I updated specification and implementation accordingly.
Comment 70 IngridvdM 2009-03-16 15:32:17 UTC
I created issue 100231 to adapt the automatic test scripts for this feature. This needs to be done within the same CWS as the feature implementation.
Comment 71 IngridvdM 2009-03-17 13:20:06 UTC
I found that after the interface redesign the import from xls did not work anymore. I fixed this now. I also added the import from xlsx.
Comment 72 IngridvdM 2009-04-23 15:11:35 UTC
@kla, please verify in CWS koheichart01.
Comment 73 kla 2009-05-18 16:05:37 UTC
Created attachment 62346 [details] TCS Display_of_hidden_cells.htm
Comment 74 kla 2009-05-18 16:08:08 UTC
Issue verified, TCS written
Comment 75 Regina Henschel 2009-05-30 20:07:03 UTC
*** Issue 102369 has been marked as a duplicate of this issue. ***
Comment 76 amy2008 2009-08-13 07:09:44 UTC
Verified in DEV300m54 on Ubuntu. Closing