GNOME Bugzilla – Bug 364291
Bad dialog behaviour after deleting a target sheet in a new view.
Last modified: 2013-03-04 00:58:13 UTC
Steps to reproduce: - View > New View - Press OK - In the first view, Tools > Scenarios > Add - In the second view, right-click on the Sheet1 tab and remove Sheet1 - In the first view, name the scenario 'a' and press OK - Press OK (on the "Invalid changing cells" message box) Backtrace: Program received signal SIGSEGV, Segmentation fault.
+ Trace 77767
Thread NaN (LWP 2164)
Ok, thanks. There is no point looking for more cases like this -- we need a general solution to this.
I think we need gui-util.c function taking a dialog and a set of flags specifying under what conditions the dialog should be destroyed. * Sheet added * Sheet deleted * Sheets reordered * Sheet renamed * ... If anything like this is done underneath the dialog, it goes *poof*. Which is far better than a crash. I imagine that most dialogs should be killed if the current sheet is killed. And that renames and reordering are mostly fine.
Ok, gnm_dialog_setup_destroy_handlers is in place and the scenarios->add dialog uses it (and this crash is thus history). Leaving open since, surely, most other dialog will need a suitable call to gnm_dialog_setup_destroy_handlers too. Lowering severity. This bug reminds me a little bit of TeX' "Intervowen alignment preables are not allowed." error message. You're invited to look up the manual's explanation of that.
I added handlers for simulation search-and-replace solver tabulate zoom
Note: we can crash every analysis tool by directing the output to the sheet that is being deleted. These dialogs are typically resilient against deletion of a sheet from which the input is taken.
The analysis tools should be okay now.
Are tehre any problematic dialog left or is this bug really fixed in svn??
Not fixed, but requires a hunt for victims. "Goto cell" dialog is an example.
Cell format dialog too. (See comment #3 for the remedy.)
I was hoping that maybe sum1 views this as a challenge to find all dialogs that are still affected. But of course we can check for which dialogs we seem to have and delete those ones that already have destroy handlers in place. That leaves us with: dialog-about.c should not be affected dialog-advanced-filter.c uses dialog_tool_init dialog-analysis-tool-frequency.c uses dialog_tool_init dialog-analysis-tool-kaplan-meier.c uses dialog_tool_init dialog-analysis-tools.c done (in dialog_tool_init) dialog-autocorrect.c dialog-autofilter.c dialog-autoformat.c dialog-autosave.c dialog-cell-comment.c dialog-cell-format.c dialog-cell-sort.c dialog-col-row.c dialog-col-width.c dialog-consolidate.c uses dialog_tool_init dialog-data-table.c dialog-define-names.c dialog-delete-cells.c dialog-doc-metadata.c dialog-fill-series.c uses dialog_tool_init dialog-formula-guru.c dialog-function-select.c dialog-goal-seek.c dialog-goto-cell.c dialog-hyperlink.c dialog-insert-cells.c dialog-merge.c dialog-password.c dialog-paste-names.c dialog-paste-special.c dialog-pivottable.c dialog-plugin-manager.c should not be affected dialog-preferences.c should not be affected dialog-printer-setup.c dialog-quit.c should not be affected dialog-random-generator.c uses dialog_tool_init dialog-recent.c should not be affected dialog-row-height.c dialog-scenarios.c done (and uses dialog_tool_init ?) dialog-search.c done dialog-search-replace.c done dialog-sheet-order.c should not be affected dialog-shuffle.c uses dialog_tool_init dialog-simulation.c done (and uses dialog_tool_init ?) dialog-so-list.c dialog-solver.c done dialog-so-styled.c dialog-stf.c dialog-stf-csv-page.c dialog-stf-export.c dialog-stf-fixed-page.c dialog-stf-format-page.c dialog-stf-main-page.c dialog-stf-preview.c dialog-tabulate.c done dialog-view.c dialog-workbook-attr.c should not be affected dialog-zoom.c done
It's actually quite difficult to trigger since the undo system generally holds on to the sheets. But here's how: 1. Append new sheet. 2. Open new view. 3. Select new sheet and open dialog in question. 4. In other view, Undo and enter "xxx" in a cell. 5. Play with dialog. (4) moves the sheet to the redo queue which is then cleared.
"Goto cell" dialog should be save now.
"cell-format" dialog should be save now.
cell comment dialog done. (Note: I used GNM_DIALOG_DESTROY_CURRENT_SHEET_REMOVED even though that is currently implemented as GNM_DIALOG_DESTROY_SHEET_REMOVED.) We need to do anything that stores a sheet in the state.
It is not just about storing a sheet in the state. Many dialogs validate the input on the fly and disable/enable the okay buttons (for example all the analysis tools). If a sheet is renamed (or deleted) the validation result would change so ideally revaildation should happen.
new overview: dialog-autocorrect.c: not affected dialog-autofilter.c dialog-autoformat.c dialog-autosave.c: not affected dialog-cell-comment.c: fixed by Morten dialog-cell-format.c: fixed dialog-cell-sort.c dialog-col-row.c: not affected but needs extra work since we just apply grouping to whatwever sheet happens to be at the top. dialog-col-width.c: fixed dialog-data-table.c dialog-define-names.c dialog-delete-cells.c dialog-doc-metadata.c dialog-formula-guru.c dialog-function-select.c dialog-goal-seek.c dialog-goto-cell.c: fixed dialog-hyperlink.c dialog-insert-cells.c dialog-merge.c dialog-password.c: not affected dialog-paste-names.c dialog-paste-special.c dialog-pivottable.c dialog-printer-setup.c dialog-row-height.c: fixed dialog-so-list.c dialog-so-styled.c dialog-stf.c dialog-stf-csv-page.c dialog-stf-export.c dialog-stf-fixed-page.c dialog-stf-format-page.c dialog-stf-main-page.c dialog-stf-preview.c dialog-view.c
dialog-autofilter.c: fixed dialog-autoformat.c: not affected (blocks all views) dialog-cell-sort.c: fixed dialog-printer-setup.c: fixed
dialog-delete-cells.c: fixed dialog-hyperlink.c: fixed dialog-view.c: not affected dialog-merge.c: fixed
new overview: dialog-data-table.c dialog-define-names.c dialog-doc-metadata.c dialog-formula-guru.c dialog-function-select.c dialog-goal-seek.c dialog-insert-cells.c dialog-paste-names.c dialog-paste-special.c: fixed dialog-pivottable.c: not enabled yet dialog-so-list.c dialog-so-styled.c dialog-stf.c: not affected dialog-stf-csv-page.c: not affected dialog-stf-export.c: not affected dialog-stf-fixed-page.c: not affected dialog-stf-format-page.c: not affected dialog-stf-main-page.c: not affected dialog-stf-preview.c: not affected
Note that dialog-paste-names.c can currently not be reached and is incomplete anyways. So the dialogs left to worry about are: dialog-data-table.c dialog-define-names.c dialog-doc-metadata.c dialog-formula-guru.c dialog-function-select.c dialog-goal-seek.c dialog-insert-cells.c dialog-so-list.c dialog-so-styled.c where dialog-doc-metadata.c is only trivially affected: the data shown on the statistics page may become incorrect.
+dialog-goto-cell.c [I think] -dialog-goal-seek.c
-dialog-goto-cell.c -- it's ok
-dialog-define-names.c
-dialog-insert-cells.c -dialog-so-list.c -dialog-so-styled.c -dialog-data-table.c -dialog-formula-guru.c -dialog-function-select.c I'm not sure dialog-doc-metadata.c needs anything. This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.