Ensure "Title Missing" shows for appropriate books (BL-16027)

Author: StephenMcConnel

src/BloomBrowserUI/collectionsTab/BooksOfCollection.tsx

@@ -4,7 +4,7 @@ import Grid from "@mui/material/Grid";
 import * as React from "react";
 import { useState, useEffect } from "react";
 import "./BooksOfCollection.less";
-import { useApiData, useWatchApiData } from "../utils/bloomApi";
+import { post, useApiData, useWatchApiData } from "../utils/bloomApi";
 import {
     BookButton,
     bookButtonHeight,
@@ -86,6 +86,17 @@ export const BooksOfCollection: React.FunctionComponent<{
         setReloadParameter("");
     }, [unfilteredBooks, props.filter]);
 
+    // Notify the backend that the collection pane is ready to receive book label updates.
+    // Using a ref callback so this happens after the DOM is rendered, without needing useEffect.
+    const collectionPaneRef = React.useCallback(
+        (node: HTMLDivElement | null) => {
+            if (node && props.isEditableCollection && books.length > 0) {
+                post("collections/collectionPaneReady");
+            }
+        },
+        [props.isEditableCollection, books.length],
+    );
+
     //const selectedBookInfo = useMonitorBookSelection();
     const collection: ICollection = useApiData(
         `collections/collectionProps?${collectionQuery}`,
@@ -123,6 +134,7 @@ export const BooksOfCollection: React.FunctionComponent<{
             key={"BookCollection-" + props.collectionId}
             className="bookButtonPane"
             style={{ cursor: "context-menu" }}
+            ref={collectionPaneRef}
         >
             {books.length > 0 && (
                 <Grid

src/BloomExe/Book/Book.cs

@@ -328,15 +328,7 @@ public virtual string TitleBestForUserDisplay
         /// </remarks>
         public virtual string NameBestForUserDisplay
         {
-            get
-            {
-                if (BookInfo.FileNameLocked)
-                {
-                    // The user has explicitly chosen a name to use for the book, distinct from its titles.
-                    return Path.GetFileName(FolderPath);
-                }
-                return TitleBestForUserDisplay;
-            }
+            get { return BookInfo.GetBestDisplayTitle(null, this); }
         }
 
         /// <summary>

src/BloomExe/Book/BookInfo.cs

@@ -9,6 +9,7 @@
 using System.Threading;
 using System.Windows.Forms;
 using Bloom.Api;
+using Bloom.Collection;
 using Bloom.Edit;
 using Bloom.Utils;
 using L10NSharp;
@@ -341,6 +342,8 @@ public string Copyright
         /// </summary>
         public string QuickTitleUserDisplay => FolderName;
 
+        public string ThumbnailLabel;
+
         public bool TryGetPremadeThumbnail(out Image image)
         {
             string path = Path.Combine(FolderPath, "thumbnail.png");
@@ -851,9 +854,66 @@ internal string GetBestTitleForUserDisplay(
             List<string> langCodes,
             bool ignoreFolderName = false
         )
+        {
+            return GetBestDisplayTitle(null, null, langCodes, ignoreFolderName);
+        }
+
+        internal string GetBestDisplayTitle(
+            CollectionSettings settings, // may be null
+            Book book, // may be null, but if we have it, we can use it to get the title without having to read the html again
+            List<string> langCodes = null,
+            bool ignoreFolderName = false
+        )
         {
             if (!ignoreFolderName && FileNameLocked)
+            {
+                // The user has explicitly chosen a name to use for the book, distinct from its titles.
                 return FolderName;
+            }
+            if (IsInEditableCollection)
+            {
+                // If we have the book, we can use that for getting the title.
+                // If we have the settings but not the book, we can use the settings to get the title from the html.
+                if (!string.IsNullOrEmpty(FolderPath) && (book != null || settings != null))
+                {
+                    // Use the loaded book if we have it to get the best title.
+                    if (FolderPath == book?.FolderPath && book.BookInfo.SaveContext == SaveContext)
+                    {
+                        if (langCodes == null)
+                            langCodes = book.BookData.GetBasicBookLanguageCodes().ToList();
+                        return Book.GetBestTitleForDisplay(
+                            book.BookData.GetMultiTextVariableOrEmpty("bookTitle"),
+                            langCodes,
+                            IsInEditableCollection
+                        );
+                    }
+                    else
+                    {
+                        // If we can create a BookData, we can get the title from there.
+                        var htmlPath = BookStorage.FindBookHtmlInFolder(FolderPath);
+                        if (
+                            !string.IsNullOrEmpty(htmlPath)
+                            && RobustFile.Exists(htmlPath)
+                            && settings != null
+                        )
+                        {
+                            var dom = HtmlDom.CreateFromHtmlFile(htmlPath);
+                            var bookData = new BookData(dom, settings, null);
+                            if (langCodes == null)
+                                langCodes = bookData.GetBasicBookLanguageCodes().ToList();
+                            return Book.GetBestTitleForDisplay(
+                                bookData.GetMultiTextVariableOrEmpty("bookTitle"),
+                                langCodes,
+                                IsInEditableCollection
+                            );
+                        }
+                    }
+                }
+            }
+            // If we still don't have a title, try to get one from the metadata.
+            // This code is used for all books that are not in the editable collection, and for any books
+            // in the editable collection that don't seem to have any title in the HTML.  (The latter case
+            // may be hopeless, but this check isn't very expensive.)
             try
             {
                 // JSON parsing requires newlines to be double quoted with backslashes inside string values.
@@ -867,13 +927,20 @@ internal string GetBestTitleForUserDisplay(
                 // behave as expected.
                 foreach (var lang in langs.Where((l) => l != "item"))
                     multiText[lang] = titles[lang].Trim();
-                return Book.GetBestTitleForDisplay(multiText, langCodes, IsInEditableCollection);
+                if (langCodes == null)
+                    langCodes = langs.Where((l) => l != "item").ToList();
+                if (langCodes.Count > 0)
+                    return Book.GetBestTitleForDisplay(
+                        multiText,
+                        langCodes,
+                        IsInEditableCollection
+                    );
             }
             catch (Exception e)
             {
                 Console.WriteLine(e);
             }
-            return Title;
+            return Title; // Title may be empty, but we have no better option at this point.
         }
 
         private static void SafelyAddToIdSet(

src/BloomExe/Book/BookStorage.cs

@@ -1844,9 +1844,9 @@ public void SetBookName(string name)
                 Debug.Fail("(debug mode only): could not rename the folder");
             }
 
-            RaiseBookRenamedEvent(fromToPair);
-
             OnFolderPathChanged();
+
+            RaiseBookRenamedEvent(fromToPair);
         }
 
         // Move a file, possibly only changing the case of the name.
@@ -1914,9 +1914,8 @@ public void RestoreBookName(string restoredName)
             string restoredPath = Path.Combine(Path.GetDirectoryName(FolderPath), restoredName);
             var fromToPair = new KeyValuePair<string, string>(FolderPath, restoredPath);
             FolderPath = restoredPath;
-            RaiseBookRenamedEvent(fromToPair);
-
             OnFolderPathChanged();
+            RaiseBookRenamedEvent(fromToPair);
         }
 
         private void RaiseBookRenamedEvent(KeyValuePair<string, string> fromToPair)

src/BloomExe/Collection/BookCollection.cs

@@ -24,7 +24,11 @@ public enum CollectionType
             SourceCollection,
         }
 
-        public delegate BookCollection Factory(string path, CollectionType collectionType); //autofac uses this
+        public delegate BookCollection Factory(
+            string path,
+            CollectionType collectionType,
+            CollectionSettings collectionSettings = null
+        ); //autofac uses this
 
         public EventHandler CollectionChanged;
 
@@ -37,6 +41,8 @@ public enum CollectionType
         private static HashSet<string> _changingFolders = new HashSet<string>();
         private BloomWebSocketServer _webSocketServer;
 
+        private CollectionSettings _collectionSettings;
+
         public static event EventHandler CollectionCreated;
 
         //for moq only
@@ -53,13 +59,15 @@ public BookCollection(
             CollectionType collectionType,
             BookSelection bookSelection,
             TeamCollectionManager tcm = null,
+            CollectionSettings collectionSettings = null,
             BloomWebSocketServer webSocketServer = null
         )
         {
             _path = path;
             _bookSelection = bookSelection;
             _tcManager = tcm;
             _webSocketServer = webSocketServer;
+            _collectionSettings = collectionSettings;
 
             Type = collectionType;
 
@@ -239,11 +247,13 @@ public void DeleteBook(Book.BookInfo bookInfo)
         /// <param name="bookInfo"></param>
         public void HandleBookDeletedFromCollection(string folderPath)
         {
-            var infoToDelete = _bookInfos.FirstOrDefault(b => b.FolderPath == folderPath);
-            //Debug.Assert(_bookInfos.Contains(bookInfo)); this will occur if we delete a book from the BloomLibrary section
-            if (infoToDelete != null) // for paranoia. We shouldn't be trying to delete a book that isn't there.
-                _bookInfos.Remove(infoToDelete);
-
+            lock (_bookInfoLock)
+            {
+                var infoToDelete = _bookInfos.FirstOrDefault(b => b.FolderPath == folderPath);
+                //Debug.Assert(_bookInfos.Contains(bookInfo)); this will occur if we delete a book from the BloomLibrary section
+                if (infoToDelete != null) // for paranoia. We shouldn't be trying to delete a book that isn't there.
+                    _bookInfos.Remove(infoToDelete);
+            }
             if (CollectionChanged != null)
                 CollectionChanged.Invoke(this, null);
         }
@@ -325,28 +335,34 @@ public string PathToDirectory
 
         public void UpdateBookInfo(BookInfo info)
         {
-            var oldIndex = _bookInfos.FindIndex(i => i.Id == info.Id);
-            IComparer<string> comp = new NaturalSortComparer<string>();
-            var newKey = Path.GetFileName(info.FolderPath);
-            if (oldIndex >= 0)
+            lock (_bookInfoLock)
             {
-                // optimize: very often the new one will belong at the same index,
-                // if that's the case we could just replace.
-                _bookInfos.RemoveAt(oldIndex);
-            }
+                var oldIndex = _bookInfos.FindIndex(i => i.Id == info.Id);
+                IComparer<string> comp = new NaturalSortComparer<string>();
+                var newKey = Path.GetFileName(info.FolderPath);
+                if (oldIndex >= 0)
+                {
+                    // optimize: very often the new one will belong at the same index,
+                    // if that's the case we could just replace.
+                    _bookInfos.RemoveAt(oldIndex);
+                }
 
-            int newIndex = _bookInfos.FindIndex(x =>
-                comp.Compare(newKey, Path.GetFileName(x.FolderPath)) <= 0
-            );
-            if (newIndex < 0)
-                newIndex = _bookInfos.Count;
-            _bookInfos.Insert(newIndex, info);
+                int newIndex = _bookInfos.FindIndex(x =>
+                    comp.Compare(newKey, Path.GetFileName(x.FolderPath)) <= 0
+                );
+                if (newIndex < 0)
+                    newIndex = _bookInfos.Count;
+                _bookInfos.Insert(newIndex, info);
+            }
             NotifyCollectionChanged();
         }
 
         public void AddBookInfo(BookInfo bookInfo)
         {
-            _bookInfos.Add(bookInfo);
+            lock (_bookInfoLock)
+            {
+                _bookInfos.Add(bookInfo);
+            }
             NotifyCollectionChanged();
         }
 
@@ -356,22 +372,25 @@ public void AddBookInfo(BookInfo bookInfo)
         /// <param name="bookInfo"></param>
         public void InsertBookInfo(BookInfo bookInfo)
         {
-            IComparer<string> comparer = new NaturalSortComparer<string>();
-            for (int i = 0; i < _bookInfos.Count; i++)
+            lock (_bookInfoLock)
             {
-                var compare = comparer.Compare(_bookInfos[i].FolderPath, bookInfo.FolderPath);
-                if (compare == 0)
-                {
-                    _bookInfos[i] = bookInfo; // Replace
-                    return;
-                }
-                if (compare > 0)
+                IComparer<string> comparer = new NaturalSortComparer<string>();
+                for (int i = 0; i < _bookInfos.Count; i++)
                 {
-                    _bookInfos.Insert(i, bookInfo);
-                    return;
+                    var compare = comparer.Compare(_bookInfos[i].FolderPath, bookInfo.FolderPath);
+                    if (compare == 0)
+                    {
+                        _bookInfos[i] = bookInfo; // Replace
+                        return;
+                    }
+                    if (compare > 0)
+                    {
+                        _bookInfos.Insert(i, bookInfo);
+                        return;
+                    }
                 }
+                _bookInfos.Add(bookInfo);
             }
-            _bookInfos.Add(bookInfo);
         }
 
         private bool BackupFileExists(string folderPath)
@@ -412,7 +431,10 @@ private void AddBookInfo(string folderPath)
                     )
                         ? _bookSelection.CurrentSelection.BookInfo
                         : new BookInfo(folderPath, editable, sc);
-
+                bookInfo.ThumbnailLabel = bookInfo.GetBestDisplayTitle(
+                    _collectionSettings,
+                    _bookSelection.CurrentSelection
+                );
                 _bookInfos.Add(bookInfo);
             }
             catch (Exception e)
@@ -500,7 +522,10 @@ private void WatcherOnChange(object sender, FileSystemEventArgs fileSystemEventA
         {
             if (_watcherIsDisabled)
                 return;
-            _bookInfos = null; // Possibly obsolete; next request will update it.
+            lock (_bookInfoLock)
+            {
+                _bookInfos = null; // Possibly obsolete; next request will update it.
+            }
             DebounceFolderChanged(fileSystemEventArgs.FullPath);
         }
 

src/BloomExe/CollectionTab/CollectionModel.cs

@@ -360,7 +360,8 @@ private IEnumerable<BookCollection> GetBookCollectionsOnce()
             {
                 editableCollection = _bookCollectionFactory(
                     _pathToCollection,
-                    BookCollection.CollectionType.TheOneEditableCollection
+                    BookCollection.CollectionType.TheOneEditableCollection,
+                    _collectionSettings
                 );
                 if (_bookCollectionHolder != null)
                     _bookCollectionHolder.TheOneEditableCollection = editableCollection;