Spreadsheet: a hidden [details] column and a registration point for objects the ordinary columns cannot carry

src/BloomExe/Spreadsheet/InternalSpreadsheet.cs

@@ -39,6 +39,16 @@ public class InternalSpreadsheet
         public const string WidgetSourceColumnLabel = "[activities source]";
         public const string PageTypeColumnLabel = "[page type]";
         public const string AttributeColumnLabel = "[attribute]";
+
+        // A hidden column holding a JSON object with whatever non-textual state a row's
+        // object needs to be reconstructed on import. Which object a row is for is said by
+        // the "kind" property of that JSON, matching the Kind of a registered
+        // ISpreadsheetObjectKind. Its presence anywhere in a spreadsheet marks that
+        // spreadsheet as the authority on the objects whose rows use it; a spreadsheet
+        // without the column (e.g. made by an older Bloom) can say nothing about them, so
+        // import leaves whatever the book already has alone.
+        public const string DetailsColumnLabel = "[details]";
+        public const string DetailsColumnFriendlyName = "Details";
         public const string ImageSourceColumnFriendlyName = "Image File Path";
 
         public const string BlankContentIndicator = "[blank]";
@@ -350,7 +360,10 @@ public int AddColumnForTag(
             {
                 GetColumnForTag(PageNumberColumnLabel),
                 GetColumnForTag(ImageSourceColumnLabel),
-            };
+                GetColumnForTag(DetailsColumnLabel),
+            }
+                .Where(i => i >= 0) // optional columns may be absent
+                .ToList();
 
         public void SortHiddenContentRowsToTheBottom()
         {

src/BloomExe/Spreadsheet/SpreadsheetExporter.cs

@@ -183,42 +183,91 @@ private void AddContentRows(
             Color colorForPage
         )
         {
-            var imageContainers = GetImageContainersAndBloomCanvases(page);
+            // Anything inside one of these objects belongs to the object, which gets rows
+            // of its own below; it must not also become a [page content] row, or a single
+            // object holding many text boxes would fill the sheet with positional rows that
+            // no importer could put back where they came from.
+            var objects = SpreadsheetObjectKinds.ObjectsOnPage(page);
+            var imageContainers = GetImageContainersAndBloomCanvases(page)
+                .Where(x => !SpreadsheetObjectKinds.IsInsideAnObject(x))
+                .ToList();
             var allGroups = TranslationGroupManager.SortedGroupsOnPage(page, true);
             var groups = allGroups
-                .Where(x => !x.GetAttribute("class").Contains("bloom-imageDescription"))
+                .Where(x =>
+                    !x.GetAttribute("class").Contains("bloom-imageDescription")
+                    && !SpreadsheetObjectKinds.IsInsideAnObject(x)
+                )
                 .ToList();
             var videoContainers = page.SafeSelectNodes(
                     ".//*[contains(@class,'bloom-videoContainer')]"
                 )
                 .Cast<SafeXmlElement>()
+                .Where(x => !SpreadsheetObjectKinds.IsInsideAnObject(x))
                 .ToList();
             var widgetContainers = page.SafeSelectNodes(
                     ".//*[contains(@class,'bloom-widgetContainer')]"
                 )
                 .Cast<SafeXmlElement>()
+                .Where(x => !SpreadsheetObjectKinds.IsInsideAnObject(x))
                 .ToList();
 
             var pageType = SpreadsheetImporter.GetLabelFromPage(page);
 
+            // Puts the page type on the first row we make for the page, whatever kind of row
+            // that is. If we make more rows for this page, we don't specify a type; that
+            // allows subsequent rows to go onto the same page if there is room.
+            void SetPageTypeIfNeeded(ContentRow rowNeedingType)
+            {
+                if (string.IsNullOrEmpty(pageType))
+                    return;
+                var pageTypeIndex = _spreadsheet.AddColumnForTag(
+                    InternalSpreadsheet.PageTypeColumnLabel,
+                    "Page Type"
+                );
+                rowNeedingType.SetCell(pageTypeIndex, pageType);
+                pageType = null;
+            }
+
             // Each of these will result in one row in the output.
-            var rowContentSources = Extensions.MapUnevenLists(
+            var rowContentSources = Extensions
+                .MapUnevenLists(
+                    new[] { groups, imageContainers, videoContainers, widgetContainers }
+                )
+                .ToList();
+
+            // Where each object's rows go among the page's [page content] rows: after as
+            // many of them as hold something that precedes the object in the document.
+            var pageContentRowsBeforeObject = RowsBeforeEachObject(
+                page,
+                objects,
                 new[] { groups, imageContainers, videoContainers, widgetContainers }
             );
-            foreach (var pageContent in rowContentSources)
+            var objectsWritten = 0;
+            void WriteObjectsDueBefore(int pageContentRowsWritten)
             {
-                var row = new ContentRow(_spreadsheet);
-                if (!string.IsNullOrEmpty(pageType))
+                while (
+                    objectsWritten < objects.Count
+                    && pageContentRowsBeforeObject[objectsWritten] <= pageContentRowsWritten
+                )
                 {
-                    var pageTypeIndex = _spreadsheet.AddColumnForTag(
-                        InternalSpreadsheet.PageTypeColumnLabel,
-                        "Page Type"
+                    ExportObjectRows(
+                        objects[objectsWritten],
+                        pageNumber,
+                        colorForPage,
+                        bookFolderPath,
+                        SetPageTypeIfNeeded
                     );
-                    row.SetCell(pageTypeIndex, pageType);
-                    // If we make more rows for this page, don't specify a type.
-                    // This allows subsequent rows to go onto the same page if there is room.
-                    pageType = null;
+                    objectsWritten++;
                 }
+            }
+
+            var pageContentRowsSoFar = 0;
+            foreach (var pageContent in rowContentSources)
+            {
+                WriteObjectsDueBefore(pageContentRowsSoFar);
+                pageContentRowsSoFar++;
+                var row = new ContentRow(_spreadsheet);
+                SetPageTypeIfNeeded(row);
                 row.SetCell(
                     InternalSpreadsheet.RowTypeColumnLabel,
                     InternalSpreadsheet.PageContentRowLabel
@@ -279,6 +328,72 @@ Color colorForPage
 
                 row.BackgroundColor = colorForPage;
             }
+            // Any objects that come after everything else on the page, including the case of
+            // a page whose only content is such an object, where there are no [page content]
+            // rows at all.
+            WriteObjectsDueBefore(int.MaxValue);
+        }
+
+        /// <summary>
+        /// For each object, how many [page content] rows must be written before it: the
+        /// largest number of items that precede it in document order in any one of the
+        /// lists those rows are made from. (Row k holds groups[k], imageContainers[k] and so
+        /// on, so everything before the object has been written once we have written that
+        /// many rows.)
+        /// </summary>
+        private static List<int> RowsBeforeEachObject(
+            SafeXmlElement page,
+            List<SpreadsheetObjectOnPage> objects,
+            List<SafeXmlElement>[] rowContentLists
+        )
+        {
+            if (objects.Count == 0)
+                return new List<int>();
+            var documentOrder = SpreadsheetObjectKinds.GetDocumentOrder(page);
+            int OrderOf(SafeXmlElement element) =>
+                documentOrder.TryGetValue(element, out var order) ? order : int.MaxValue;
+            return objects
+                .Select(objectOnPage =>
+                {
+                    var objectOrder = OrderOf(objectOnPage.Element);
+                    return rowContentLists
+                        .Select(list => list.Count(item => OrderOf(item) < objectOrder))
+                        .Max();
+                })
+                .ToList();
+        }
+
+        /// <summary>
+        /// Writes the rows for one object of a registered ISpreadsheetObjectKind, at the
+        /// point in the page's rows where the object belongs. All this generic code does is
+        /// make sure the hidden [details] column exists -- its presence is what tells the
+        /// importer that this spreadsheet is the authority on such objects -- and then let
+        /// the kind write whatever rows it needs.
+        /// </summary>
+        private void ExportObjectRows(
+            SpreadsheetObjectOnPage objectOnPage,
+            string pageNumber,
+            Color colorForPage,
+            string bookFolderPath,
+            Action<ContentRow> setPageTypeIfNeeded
+        )
+        {
+            _spreadsheet.AddColumnForTag(
+                InternalSpreadsheet.DetailsColumnLabel,
+                InternalSpreadsheet.DetailsColumnFriendlyName
+            );
+            objectOnPage.Kind.ExportObject(
+                objectOnPage.Element,
+                new SpreadsheetObjectExportContext
+                {
+                    Exporter = this,
+                    Spreadsheet = _spreadsheet,
+                    PageNumber = pageNumber,
+                    ColorForPage = colorForPage,
+                    BookFolderPath = bookFolderPath,
+                    SetPageTypeIfNeeded = setPageTypeIfNeeded,
+                }
+            );
         }
 
         private void WriteWidget(
@@ -341,7 +456,12 @@ string bookFolderPath
             }
         }
 
-        private void WriteVideo(
+        /// <summary>
+        /// Writes a bloom-videoContainer's video into the row's [video source] cell,
+        /// copying the file into the spreadsheet's video folder. internal so that an
+        /// ISpreadsheetObjectKind can put a video inside its object into a row of its own.
+        /// </summary>
+        internal void WriteVideo(
             SafeXmlElement videoContainer,
             ContentRow row,
             string bookFolderPath
@@ -389,7 +509,13 @@ string bookFolderPath
             }
         }
 
-        private void WriteTranslationGroup(
+        /// <summary>
+        /// Writes each language's text of a bloom-translationGroup into that language's
+        /// column of the row (plus any audio columns). internal so that an
+        /// ISpreadsheetObjectKind can put text inside its object into a row of its own, and
+        /// so a translator sees that text in the same language columns as any other text.
+        /// </summary>
+        internal void WriteTranslationGroup(
             SafeXmlElement translationGroup,
             ContentRow row,
             string bookFolderPath
@@ -634,7 +760,12 @@ private List<SafeXmlElement> GetImageContainersAndBloomCanvases(SafeXmlElement e
                 .ToList();
         }
 
-        private string ImagePath(string imagesFolderPath, string imageSrc)
+        /// <summary>
+        /// The full path of an image, given the folder it is in and the (url-encoded) src
+        /// attribute that referred to it. internal for the use of ISpreadsheetObjectKind
+        /// implementations that export images inside their objects.
+        /// </summary>
+        internal string ImagePath(string imagesFolderPath, string imageSrc)
         {
             return Path.Combine(
                 imagesFolderPath,
@@ -840,7 +971,12 @@ ref imageSrcAttribute
             }
         }
 
-        private void CopyImageFileToSpreadsheetFolder(string imageSourcePath)
+        /// <summary>
+        /// Copies an image the book uses into the spreadsheet's images folder, so that the
+        /// spreadsheet is self-contained. internal for the use of ISpreadsheetObjectKind
+        /// implementations that export images inside their objects.
+        /// </summary>
+        internal void CopyImageFileToSpreadsheetFolder(string imageSourcePath)
         {
             if (_outputImageFolder != null)
             {

src/BloomExe/Spreadsheet/SpreadsheetIO.cs

@@ -366,6 +366,21 @@ private static bool IsWysiwygFormattedPair(SpreadsheetRow row, int index)
             return IsWysiwygFormattedColumn(row, index) && IsWysiwygFormattedRow(row);
         }
 
+        /// <summary>
+        /// True for a cell of the [details] column, whose JSON we must get back byte for
+        /// byte. Such a cell gets neither XML escaping nor the decoding of Excel's _xNNNN_
+        /// character escapes: text that merely looks like such an escape (say, "_x0041_" in
+        /// a label) must stay as it is, not become the character it would name. Excel needs
+        /// those escapes only for characters XML cannot hold, and JSON never holds one raw.
+        /// </summary>
+        private static bool IsCopiedVerbatim(SpreadsheetRow row, int index)
+        {
+            if (row.Spreadsheet.AllRows().Count() <= 1)
+                return false; // the header row itself is being read; see WantXmlEscaping
+            return row.Spreadsheet.Header.GetRow(0).GetCell(index).Content
+                == InternalSpreadsheet.DetailsColumnLabel;
+        }
+
         private static bool WantXmlEscaping(SpreadsheetRow row, int index)
         {
             if (row.Spreadsheet.AllRows().Count() <= 1)
@@ -379,8 +394,12 @@ private static bool WantXmlEscaping(SpreadsheetRow row, int index)
             // PageType column holds what is basically the InnerText of an element;
             // the InnerText property handles any XML escaping.
             // Image source column holds the actual path to the file; it will be url encoded when set as the src.
+            // The details column holds JSON, which we must get back byte for byte: escaping
+            // its ampersands (e.g. in a url inside an attribute value) would corrupt what we
+            // are copying verbatim.
             return key != InternalSpreadsheet.PageTypeColumnLabel
-                && key != InternalSpreadsheet.ImageSourceColumnLabel;
+                && key != InternalSpreadsheet.ImageSourceColumnLabel
+                && key != InternalSpreadsheet.DetailsColumnLabel;
         }
 
         private static bool IsWysiwygFormattedColumn(SpreadsheetRow row, int index)
@@ -392,6 +411,7 @@ private static bool IsWysiwygFormattedColumn(SpreadsheetRow row, int index)
                 || key == InternalSpreadsheet.WidgetSourceColumnLabel
                 || key == InternalSpreadsheet.PageTypeColumnLabel
                 || key == InternalSpreadsheet.AttributeColumnLabel
+                || key == InternalSpreadsheet.DetailsColumnLabel
             )
                 return false;
             return !nonWysiwygColumns.Contains(key);
@@ -438,13 +458,16 @@ SpreadsheetRow row
                 }
                 else
                 {
-                    var cellContent = worksheet.Cells[rowIndex + 1, c + 1].Value ?? "";
-                    row.AddCell(
-                        ReplaceExcelEscapedCharsAndEscapeXmlOnes(
-                            cellContent.ToString(),
-                            WantXmlEscaping(row, c)
-                        )
-                    );
+                    var cellContent = (worksheet.Cells[rowIndex + 1, c + 1].Value ?? "").ToString();
+                    if (IsCopiedVerbatim(row, c))
+                        row.AddCell(cellContent);
+                    else
+                        row.AddCell(
+                            ReplaceExcelEscapedCharsAndEscapeXmlOnes(
+                                cellContent,
+                                WantXmlEscaping(row, c)
+                            )
+                        );
                 }
             }
         }