]> git.lyx.org Git - features.git/commitdiff
Possible fix for the mystery crash, which is bug #9049.
authorRichard Heck <rgheck@lyx.org>
Thu, 7 Aug 2014 19:00:35 +0000 (15:00 -0400)
committerRichard Heck <rgheck@lyx.org>
Fri, 15 Aug 2014 14:44:37 +0000 (10:44 -0400)
Investigation of bug #9236 showed that crash to be due to a Paragraph's
holding a dangling pointer to an old and deleted Layout after the
DocumentClass was reset. Since the backtraces look almost identical, it
seems likely that we have the same problem here.

Since this crash seems almost always to involve tables, I looked at the
code in switchBetweenClasses() and found that the Paragraphs that belong
to "hidden" table cells are not seen by the initial recursion using a
ParIterator: It skips right over them. This was confirmed by test code
suggested by Enrico, with results reported in Trac.

The present patch attempts to deal with this problem in the second
recursion, over Insets. When we see an InsetTabular, we call a new
routine that recurses through the cells, looking for hidden ones. If it
finds a hidden one, it then resets the Layout for the cell's Paragraphs
(there should be only one, but we do not make any assumptions) to the
PlainLayout that belongs to the new DocumentClass. This is good enough,
since such cells never have content.

There is extensive discussion of the patch here:
  https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg185095.html
Additional testing by Enrico and me confirmed the existence of the
dangling pointer.

src/CutAndPaste.cpp
src/insets/InsetTabular.cpp
src/insets/InsetTabular.h

index c3367215e3b947f47ad6266de3dbf635dbc1f63e..ad3ad7e23843b0e8f9f1f552621f535319a0ecaf 100644 (file)
@@ -737,35 +737,42 @@ void switchBetweenClasses(DocumentClassConstPtr oldone,
                        it->setLayout(newtc[name]);
        }
 
-       // character styles
+       // character styles and hidden table cells
        InsetIterator const i_end = inset_iterator_end(in);
        for (InsetIterator it = inset_iterator_begin(in); it != i_end; ++it) {
-               if (it->lyxCode() != FLEX_CODE)
+               InsetCode const code = it->lyxCode();
+               if (code == FLEX_CODE) {
                        // FIXME: Should we verify all InsetCollapsable?
-                       continue;
-
-               docstring const layoutName = it->layoutName();
-               docstring const & n = newone->insetLayout(layoutName).name();
-               bool const is_undefined = n.empty() ||
-                       n == DocumentClass::plainInsetLayout().name();
-               if (!is_undefined)
-                       continue;
-
-               // The flex inset is undefined in newtc
-               docstring const oldname = from_utf8(oldtc.name());
-               docstring const newname = from_utf8(newtc.name());
-               docstring s;
-               if (oldname == newname)
-                       s = bformat(_("Flex inset %1$s is undefined after "
-                               "reloading `%2$s' layout."), layoutName, oldname);
-               else
-                       s = bformat(_("Flex inset %1$s is undefined because of "
-                               "conversion from `%2$s' layout to `%3$s'."),
-                               layoutName, oldname, newname);
-               // To warn the user that something had to be done.
-               errorlist.push_back(ErrorItem(
-                               _("Undefined flex inset"),
-                               s, it.paragraph().id(), it.pos(), it.pos() + 1));
+                       docstring const layoutName = it->layoutName();
+                       docstring const & n = newone->insetLayout(layoutName).name();
+                       bool const is_undefined = n.empty() ||
+                               n == DocumentClass::plainInsetLayout().name();
+                       if (!is_undefined)
+                               continue;
+       
+                       // The flex inset is undefined in newtc
+                       docstring const oldname = from_utf8(oldtc.name());
+                       docstring const newname = from_utf8(newtc.name());
+                       docstring s;
+                       if (oldname == newname)
+                               s = bformat(_("Flex inset %1$s is undefined after "
+                                       "reloading `%2$s' layout."), layoutName, oldname);
+                       else
+                               s = bformat(_("Flex inset %1$s is undefined because of "
+                                       "conversion from `%2$s' layout to `%3$s'."),
+                                       layoutName, oldname, newname);
+                       // To warn the user that something had to be done.
+                       errorlist.push_back(ErrorItem(
+                                       _("Undefined flex inset"),
+                                       s, it.paragraph().id(), it.pos(), it.pos() + 1));
+               } else if (code == TABULAR_CODE) {
+                       // The recursion above does not catch paragraphs in "hidden" cells,
+                       // i.e., ones that are part of a multirow or multicolum. So we need
+                       // to handle those separately.
+                       // This is the cause of bug #9049.
+                       InsetTabular * table = it->asInsetTabular();
+                       table->setLayoutForHiddenCells(newtc);
+               }
        }
 }
 
index ac8e723d87cfc3f4a3acf5b970713ba703be1c1c..2a9c7e64b7a983f2b35150d7342b5d9f3c664814 100644 (file)
@@ -6442,4 +6442,21 @@ string InsetTabular::params2string(InsetTabular const & inset)
 }
 
 
+void InsetTabular::setLayoutForHiddenCells(DocumentClass const & dc) {
+       for (Tabular::col_type c = 0; c < tabular.ncols(); ++c) {
+               for (Tabular::row_type r = 0; r < tabular.nrows(); ++r) {
+                       if (!tabular.isPartOfMultiColumn(r,c) &&
+                           !tabular.isPartOfMultiRow(r,c))
+                               continue;
+
+                       ParagraphList & parlist = tabular.cellInset(r,c)->paragraphs();
+                       ParagraphList::iterator it = parlist.begin();
+                       ParagraphList::iterator const en = parlist.end();
+                       for (; it != en; ++it)
+                                       it->setLayout(dc.plainLayout());
+               }
+       }
+}
+
+
 } // namespace lyx
index 2763eef0b8bdeda10e998ab67d2c730cc00b3c12..0aac6420ec77fc0c17f0b3e52dda4a588815ee79 100644 (file)
@@ -969,6 +969,8 @@ public:
 
        /// Returns whether the cell in the specified row and column is selected.
        bool isCellSelected(Cursor & cur, row_type row, col_type col) const;
+       ///
+       void setLayoutForHiddenCells(DocumentClass const & dc);
        //
        // Public structures and variables
        ///