From f483d5b95a4c01a093747cc02d01b12f2fdfb960 Mon Sep 17 00:00:00 2001 From: Hatton Date: Mon, 29 Apr 2024 17:21:02 -0600 Subject: [PATCH 1/5] Just renaming --- .gitignore | 2 ++ src/BloomExe/Edit/EditingModel.cs | 18 +++++++++++++----- src/BloomExe/Edit/EditingView.cs | 4 ++-- src/BloomExe/IBrowser.cs | 6 +++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 31a7afd7d31d..a5b23c4a541e 100644 --- a/.gitignore +++ b/.gitignore @@ -179,3 +179,5 @@ src/content/.yarn/* !src/content/.yarn/releases !src/content/.yarn/sdks !src/content/.yarn/versions + +.vite/vitest/results.json diff --git a/src/BloomExe/Edit/EditingModel.cs b/src/BloomExe/Edit/EditingModel.cs index be8b18c7b631..225efc54e0d3 100644 --- a/src/BloomExe/Edit/EditingModel.cs +++ b/src/BloomExe/Edit/EditingModel.cs @@ -718,14 +718,17 @@ public void ViewVisibleNowDoSlowStuff() // completed saving it. int _selChangeCount = 0; object _selChangeLock = new object(); + private void OnPageSelectionChanging(object sender, EventArgs eventArgs) { try { lock (_selChangeLock) { - Debug.Assert(_selChangeCount == 0, - $"Multiple active OnPageSelectionChanging calls, possible empty marginBox cause? (count={_selChangeLock})"); + Debug.Assert( + _selChangeCount == 0, + $"Multiple active OnPageSelectionChanging calls, possible empty marginBox cause? (count={_selChangeLock})" + ); _selChangeCount++; } OnPageSelectionChangingInternal(sender, eventArgs); @@ -738,6 +741,7 @@ private void OnPageSelectionChanging(object sender, EventArgs eventArgs) } } } + private void OnPageSelectionChangingInternal(object sender, EventArgs eventArgs) { CheckForBL2634("start of page selection changing--should have old IDs"); @@ -1255,14 +1259,17 @@ private bool CannotSavePage() int _saveCount = 0; object _saveCountLock = new object(); + public void SaveNow(bool forceFullSave = false) { try { lock (_saveCountLock) { - Debug.Assert(_saveCount == 0, - $"Trying to save while already saving: possible empty marginBox cause? (save count={_saveCount})"); + Debug.Assert( + _saveCount == 0, + $"Trying to save while already saving: possible empty marginBox cause? (save count={_saveCount})" + ); _saveCount++; } SaveNowInternal(forceFullSave); @@ -1275,6 +1282,7 @@ public void SaveNow(bool forceFullSave = false) } } } + private void SaveNowInternal(bool forceFullSave = false) { if (_domForCurrentPage != null && !_inProcessOfSaving && !NavigatingSoSuspendSaving) @@ -1308,7 +1316,7 @@ private void SaveNowInternal(bool forceFullSave = false) CheckForBL2634("beginning SaveNow"); _inProcessOfSaving = true; _tasksToDoAfterSaving.Clear(); - _view.CleanHtmlAndCopyToPageDom(); + _view.GetHtmlFromBrowserAndCopyToPageDom(); //BL-1064 (and several other reports) were about not being able to save a page. The problem appears to be that //this old code: diff --git a/src/BloomExe/Edit/EditingView.cs b/src/BloomExe/Edit/EditingView.cs index bb8b94ff0abf..1a9ca0478e41 100644 --- a/src/BloomExe/Edit/EditingView.cs +++ b/src/BloomExe/Edit/EditingView.cs @@ -1485,7 +1485,7 @@ public void UpdateAllThumbnails() /// this started as an experiment, where our textareas were not being read when we saved because of the need /// to change the picture /// - public void CleanHtmlAndCopyToPageDom() + public async Task GetHtmlFromBrowserAndCopyToPageDom() { // NOTE: these calls to may lead to API calls from the JS. These are async, so the actions // that JS might perform may not actually happen until well after this method. We ran into a problem in @@ -1510,7 +1510,7 @@ public void CleanHtmlAndCopyToPageDom() userCssContent = combinedData.Substring(endHtml + "".Length); } } - _browser1.ReadEditableAreasNow(bodyHtml, userCssContent); + _browser1.ReadEditedHtmlNow(bodyHtml, userCssContent); // TODO this makes no sense being in browser. Has nothing to do with the browser. } private void _copyButton_Click(object sender, EventArgs e) diff --git a/src/BloomExe/IBrowser.cs b/src/BloomExe/IBrowser.cs index d9d833aac3f2..ff69b96e68f5 100644 --- a/src/BloomExe/IBrowser.cs +++ b/src/BloomExe/IBrowser.cs @@ -288,11 +288,11 @@ public void OnOpenPageInEdge(object sender, EventArgs e) // intentionally letting any errors just escape, give us an error } - public void ReadEditableAreasNow(string bodyHtml, string userCssContent) + public void ReadEditedHtmlNow(string bodyHtml, string userCssContent) { if (Url != "about:blank") { - LoadPageDomFromBrowser(bodyHtml, userCssContent); + LoadPageDomFromHtml(bodyHtml, userCssContent); } } @@ -302,7 +302,7 @@ public void ReadEditableAreasNow(string bodyHtml, string userCssContent) /// We're now obtaining the new content another way, so this code doesn't have any reason /// to be in this class...but we're aiming for a minimal change, maximal safety fix for 4.9 /// - private void LoadPageDomFromBrowser(string bodyHtml, string userCssContent) + private void LoadPageDomFromHtml(string bodyHtml, string userCssContent) { Debug.Assert(!InvokeRequired); if (_pageEditDom == null) From 846a5083b16d9351843c5cdc17b9a6b467693f30 Mon Sep 17 00:00:00 2001 From: Hatton Date: Mon, 29 Apr 2024 20:18:53 -0600 Subject: [PATCH 2/5] wip save via api call -- currently loses edits if click on same page --- src/BloomBrowserUI/bookEdit/editablePage.ts | 4 +- .../bookEdit/js/bloomEditing.ts | 11 +- src/BloomExe/Book/HtmlDom.cs | 7 + src/BloomExe/Edit/EditingModel.cs | 179 +++++------------- src/BloomExe/Edit/EditingView.cs | 40 +--- src/BloomExe/Edit/WebThumbNailList.cs | 2 +- src/BloomExe/Workspace/WorkspaceView.cs | 2 +- .../web/controllers/CopyrightAndLicenseApi.cs | 4 +- .../web/controllers/EditingViewApi.cs | 19 ++ .../web/controllers/SignLanguageApi.cs | 2 +- 10 files changed, 93 insertions(+), 177 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/editablePage.ts b/src/BloomBrowserUI/bookEdit/editablePage.ts index 3a326a831da4..8d07a383dd9f 100644 --- a/src/BloomBrowserUI/bookEdit/editablePage.ts +++ b/src/BloomBrowserUI/bookEdit/editablePage.ts @@ -41,7 +41,7 @@ export interface IPageFrameExports { // For example, editTabBundle.getEditablePageBundleExports().pageSelectionChanging() can be called. import { pageSelectionChanging, - getBodyContentForSavePage, + saveRequested, userStylesheetContent, pageUnloading, disconnectForGarbageCollection, @@ -54,7 +54,7 @@ import { } from "./js/bloomEditing"; export { pageSelectionChanging, - getBodyContentForSavePage, + saveRequested, userStylesheetContent, pageUnloading, disconnectForGarbageCollection, diff --git a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts index 3af34c5b7294..dc014c60b3e2 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts @@ -29,7 +29,7 @@ import "jquery.hotkeys"; //makes the on(keydown work with keynames) import "../../lib/jquery.resize"; // makes jquery resize work on all elements import { getEditTabBundleExports } from "./bloomFrames"; import { showInvisibles, hideInvisibles } from "./showInvisibles"; - +import { postJson } from "../../utils/bloomApi"; //promise may be needed to run tests with phantomjs //import promise = require('es6-promise'); //promise.Promise.polyfill(); @@ -1265,8 +1265,13 @@ export const pageSelectionChanging = () => { } }; -// Called from C# by a RunJavaScript() in EditingView.CleanHtmlAndCopyToPageDom via -// editTabBundle.getEditablePageBundleExports(). +export function saveRequested(forceFullSave: boolean) { + postJson("editView/saveHtml", { + forceFullSave: forceFullSave, + html: getBodyContentForSavePage(), + userStylesheetContent: userStylesheetContent() + }); +} export const getBodyContentForSavePage = () => { const bubbleEditingOn = theOneBubbleManager.isComicEditingOn; if (bubbleEditingOn) { diff --git a/src/BloomExe/Book/HtmlDom.cs b/src/BloomExe/Book/HtmlDom.cs index 7d2a662a8ff8..2ac1c575729a 100644 --- a/src/BloomExe/Book/HtmlDom.cs +++ b/src/BloomExe/Book/HtmlDom.cs @@ -58,6 +58,13 @@ public HtmlDom(XmlDocument domToClone) _dom = (XmlDocument)domToClone.Clone(); } + public static HtmlDom FromXmlNodeNoClone(XmlNode domToOwn) + { + var h = new HtmlDom(); + h.RawDom.DocumentElement.AppendChild(h.RawDom.ImportNode(domToOwn, true)); + return h; + } + /// /// Make a DOM out of the input /// diff --git a/src/BloomExe/Edit/EditingModel.cs b/src/BloomExe/Edit/EditingModel.cs index 225efc54e0d3..7ccbfdac88cb 100644 --- a/src/BloomExe/Edit/EditingModel.cs +++ b/src/BloomExe/Edit/EditingModel.cs @@ -158,7 +158,7 @@ ITemplateFinder sourceCollectionsList collectionClosingEvent.Subscribe(o => { if (Visible) - SaveNow(); + RequestBrowserToSave(); }); localizationChangedEvent.Subscribe(o => { @@ -166,7 +166,7 @@ ITemplateFinder sourceCollectionsList //shown so the view has never been full constructed, so we're not in a good state to do a refresh if (Visible) { - SaveNow(); + RequestBrowserToSave(); _view.UpdateButtonLocalizations(); RefreshDisplayOfCurrentPage(true); //_view.UpdateDisplay(); @@ -214,7 +214,7 @@ private void OnTabAboutToChange(TabChangedDetails details) { if (details.From == _view) { - SaveNow(); + RequestBrowserToSave(); _view.RunJavascriptAsync( "if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getEditablePageBundleExports()) !=='undefined') {editTabBundle.getEditablePageBundleExports().disconnectForGarbageCollection();}" ); @@ -310,7 +310,7 @@ private void DuplicatePageInternal(IPage page, int numberOfTimesToDuplicate = 1) { try { - SaveNow(); //ensure current page is saved first + RequestBrowserToSave(); //ensure current page is saved first _domForCurrentPage = null; //prevent us trying to save it later, as the page selection changes _currentlyDisplayedBook.DuplicatePage(page, numberOfTimesToDuplicate); // Book.DuplicatePage() updates the page list so we don't need to do it here. @@ -354,7 +354,7 @@ internal void DeletePage(IPage page) try { // BL-4035 Save any style changes to the book before deleting the page. - SaveNow(); + RequestBrowserToSave(); } catch (Exception saveError) { @@ -551,7 +551,7 @@ public IEnumerable GetSizeAndOrientationChoices() public void SetLayout(Layout layout) { - SaveNow(); + RequestBrowserToSave(); var changedOrientation = CurrentBook.GetLayout().SizeAndOrientation.IsLandScape != layout.SizeAndOrientation.IsLandScape; @@ -590,7 +590,7 @@ public void ContentLanguagesSelectionChanged() CurrentBook.SetMultilingualContentLanguages(contentLanguages); // set langs before saving page // The language choice is saved in the data-div, so we must do a full save even if this // page doesn't contain anything else that has non-local effects. - SaveNow(true); + RequestBrowserToSave(true); CurrentBook.PrepareForEditing(); RefreshDisplayOfCurrentPage(); _view.UpdatePageList(true); //counting on this to redo the thumbnails @@ -1146,7 +1146,7 @@ private void EnsureLevelAttrCorrect() _currentlyDisplayedBook.BookInfo.MetaData.LeveledReaderLevel.ToString(); if (correctLevel != currentLevel) { - SaveNow(); + RequestBrowserToSave(); _currentlyDisplayedBook.OurHtmlDom.Body.SetAttribute( "data-leveledreaderlevel", correctLevel @@ -1231,7 +1231,7 @@ private void FinishSavingPage(bool forceFullSave = false) return; var stopwatch = Stopwatch.StartNew(); - SaveNow(forceFullSave); + RequestBrowserToSave(forceFullSave); stopwatch.Stop(); Debug.WriteLine("Save Now Elapsed Time: {0} ms", stopwatch.ElapsedMilliseconds); } @@ -1257,132 +1257,49 @@ private bool CannotSavePage() private System.Windows.Forms.Timer _developerFileWatcherQuietTimer; private bool _weHaveSeenAJsonChange; - int _saveCount = 0; - object _saveCountLock = new object(); - - public void SaveNow(bool forceFullSave = false) + public void SavePageHtml(string bodyHtml, string userCssContent, bool forceFullSave) { - try - { - lock (_saveCountLock) - { - Debug.Assert( - _saveCount == 0, - $"Trying to save while already saving: possible empty marginBox cause? (save count={_saveCount})" - ); - _saveCount++; - } - SaveNowInternal(forceFullSave); - } - finally + // extract the page and convert it to xml + var dom = XmlHtmlConverter.GetXmlDomFromHtml(bodyHtml, false); + var bodyDom = dom.SelectSingleNode("//body"); + var browserDomPage = bodyDom.SelectSingleNode( + "//body//div[contains(@class,'bloom-page')]" + ); + var htmlDom = HtmlDom.FromXmlNodeNoClone(browserDomPage); + + // stick the custom styles in the head + var styles = HtmlDom.CreateUserModifiedStyles(userCssContent); + var head = htmlDom.RawDom.SelectSingleNode("//head"); + head.InnerXml = HtmlDom.CreateUserModifiedStyles(userCssContent); + + // see if there were data-div changes which require a full save + var newPageData = GetPageData(browserDomPage); + var fullSave = forceFullSave || NeedToDoFullSave(newPageData); + + // save it + _pageSelection.CurrentSelection.Book.SavePage(htmlDom, fullSave); + _pageHasUnsavedDataDerivedChange = false; + + // something to do with deleting? + while (_tasksToDoAfterSaving.Count > 0) { - lock (_saveCountLock) - { - _saveCount--; - } + var task = _tasksToDoAfterSaving[0]; + _tasksToDoAfterSaving.RemoveAt(0); + task(); } } - private void SaveNowInternal(bool forceFullSave = false) + public void RequestBrowserToSave(bool forceFullSave = false) { if (_domForCurrentPage != null && !_inProcessOfSaving && !NavigatingSoSuspendSaving) { - Logger.WriteMinorEvent("EditingModel.SaveNow() starting"); -#if MEMORYCHECK - // Check memory for the benefit of developers. - MemoryManagement.CheckMemory(false, "before EditingModel.SaveNow()", false); -#endif - try - { - _webSocketServer.SendString("pageThumbnailList", "saving", ""); - - // CleanHtml already requires that we are on UI thread. But it's worth asserting here too in case that changes. - // If we weren't sure of that we would need locking for access to _tasksToDoAfterSaving and _inProcessOfSaving, - // and would need to be careful about whether any delayed tasks needed to be on the UI thread. - if (_view.InvokeRequired) - { - NonFatalProblem.Report( - ModalIf.Beta, - PassiveIf.Beta, - "SaveNow called on wrong thread", - null - ); - _view.Invoke((Action)(() => SaveNowInternal(forceFullSave))); - Logger.WriteMinorEvent( - "EditingModel.SaveNow() finished after Invoke to get on UI thread" - ); - return; - } - CheckForBL2634("beginning SaveNow"); - _inProcessOfSaving = true; - _tasksToDoAfterSaving.Clear(); - _view.GetHtmlFromBrowserAndCopyToPageDom(); - - //BL-1064 (and several other reports) were about not being able to save a page. The problem appears to be that - //this old code: - // CurrentBook.SavePage(_domForCurrentPage); - //would some times ask book X to save a page from book Y. - //We could never reproduce it at will, so this is to help with that... - if (this._pageSelection.CurrentSelection.Book != _currentlyDisplayedBook) - { - Debug.Fail("This is the BL-1064 Situation"); - Logger.WriteEvent( - "Warning: SaveNow() with a page that is not the current book. That should be ok, but it is the BL-1064 situation (though we now work around it)." - ); - } - //but meanwhile, the page knows its book, so we can see if it looks like a valid book and give a helpful - //error if, for example, it was deleted: - try - { - if (!_pageSelection.CurrentSelection.Book.IsSaveable) - { - Logger.WriteEvent( - "Error: SaveNow() found that this book had IsSaveable=='false'" - ); - Logger.WriteEvent( - "Book path was {0}", - _pageSelection.CurrentSelection.Book.FolderPath - ); - throw new ApplicationException( - "Bloom tried to save a page to a book that was not in a position to be updated." - ); - } - } - catch (ObjectDisposedException err) // in case even calling CanUpdate gave an error - { - Logger.WriteEvent("Error: SaveNow() found that this book was disposed."); - throw err; - } - catch (Exception err) // in case even calling CanUpdate gave an error - { - Logger.WriteEvent("Error: SaveNow():CanUpdate threw an exception"); - throw err; - } - CheckForBL2634("save"); - //OK, looks safe, time to save. - var newPageData = GetPageData(_domForCurrentPage.RawDom); - _pageSelection.CurrentSelection.Book.SavePage( - _domForCurrentPage, - forceFullSave || NeedToDoFullSave(newPageData) - ); - _pageHasUnsavedDataDerivedChange = false; - CheckForBL2634("finished save"); - while (_tasksToDoAfterSaving.Count > 0) - { - var task = _tasksToDoAfterSaving[0]; - _tasksToDoAfterSaving.RemoveAt(0); - task(); - } - } - finally - { - _inProcessOfSaving = false; - } -#if MEMORYCHECK - // Check memory for the benefit of developers. - MemoryManagement.CheckMemory(false, "after EditingModel.SaveNow()", false); -#endif - Logger.WriteMinorEvent("EditingModel.SaveNow() finished"); + Logger.WriteMinorEvent("EditingModel.RequestSave() starting"); + _webSocketServer.SendString("pageThumbnailList", "saving", ""); + _tasksToDoAfterSaving.Clear(); // review + // review do we really need to be checking to see if things are loaded? If they are not, then there is nothing to save, and this doesn't thow. + var script = + $"console.log('saving'); editTabBundle.getToolboxBundleExports().removeToolboxMarkup();editTabBundle.getEditablePageBundleExports().saveRequested({forceFullSave.ToString().ToLowerInvariant()});"; + _view.Browser.RunJavascriptAsync(script); } else { @@ -1498,7 +1415,7 @@ IProgress progress ); // We need to save so that when asked by the thumbnailer, the book will give the proper image - SaveNow(); + RequestBrowserToSave(); // BL-3717: if we cleanup unused image files whenever we change a picture then Cut can lose // all of an image's metadata (because the actual file is missing from the book folder when we go to @@ -1689,14 +1606,14 @@ public void PreserveHtmlOfElement(string elementHtml) public void ShowAddPageDialog() { - SaveNow(); // At least in template mode, the current page shows in the Add Page dialog, and should be current. + RequestBrowserToSave(); // At least in template mode, the current page shows in the Add Page dialog, and should be current. _view.ShowAddPageDialog(); } internal void ChangePageLayout(IPage page) { PageChangingLayout = page; - SaveNow(); // need to preserve any typing they've done but not yet saved + RequestBrowserToSave(); // need to preserve any typing they've done but not yet saved _view.ShowChangeLayoutDialog(); } @@ -1751,7 +1668,7 @@ public bool GetClipboardHasPage() public void CopyPage(IPage page) { - SaveNow(); // need to preserve any typing they've done but not yet saved (BL-4512) + RequestBrowserToSave(); // need to preserve any typing they've done but not yet saved (BL-4512) // We have to clone this so that if the user changes the page after doing the copy, // when they paste they get the page as it was, not as it is now. _pageDivFromCopyPage = (XmlElement)page.GetDivNodeForThisPage().CloneNode(true); diff --git a/src/BloomExe/Edit/EditingView.cs b/src/BloomExe/Edit/EditingView.cs index 1a9ca0478e41..ee81d837021d 100644 --- a/src/BloomExe/Edit/EditingView.cs +++ b/src/BloomExe/Edit/EditingView.cs @@ -796,7 +796,7 @@ public void OnPasteImage(int imgIndex) { // BL-11709: It's just possible that the element we are pasting into has not yet been saved, // which will cause problems. So do a save before pasting. - _model.SaveNow(); + _model.RequestBrowserToSave(); using (var measure = PerformanceMeasurement.Global.Measure("Paste Image")) { @@ -1046,7 +1046,7 @@ public void OnChangeImage(int imgIndex) { // Make sure any new image overlays on the page are saved, so our imgIndex will select the // right one. - Model.SaveNow(); + Model.RequestBrowserToSave(); var imageElement = GetImageNode(imgIndex); if (imageElement == null) @@ -1481,38 +1481,6 @@ public void UpdateAllThumbnails() _pageListView.UpdateAllThumbnails(); } - /// - /// this started as an experiment, where our textareas were not being read when we saved because of the need - /// to change the picture - /// - public async Task GetHtmlFromBrowserAndCopyToPageDom() - { - // NOTE: these calls to may lead to API calls from the JS. These are async, so the actions - // that JS might perform may not actually happen until well after this method. We ran into a problem in - // BL-9912 where the Leveled Reader Tool was prompted by some of this to call us back with a save to the - // tool state, but by then the editingModel had cleared out its knowledge of what book it had previously - // been editing, so there was an null. - var script = - @" -if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getToolboxBundleExports()) !=='undefined') - editTabBundle.getToolboxBundleExports().removeToolboxMarkup(); -if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getEditablePageBundleExports()) !=='undefined') - editTabBundle.getEditablePageBundleExports().getBodyContentForSavePage() + '' + editTabBundle.getEditablePageBundleExports().userStylesheetContent();"; - var combinedData = RunJavascriptWithStringResult_Sync_Dangerous(script); - string bodyHtml = null; - string userCssContent = null; - if (combinedData != null) - { - var endHtml = combinedData.IndexOf("", StringComparison.Ordinal); - if (endHtml > 0) - { - bodyHtml = combinedData.Substring(0, endHtml); - userCssContent = combinedData.Substring(endHtml + "".Length); - } - } - _browser1.ReadEditedHtmlNow(bodyHtml, userCssContent); // TODO this makes no sense being in browser. Has nothing to do with the browser. - } - private void _copyButton_Click(object sender, EventArgs e) { ExecuteCommandSafely(_copyCommand); @@ -1870,7 +1838,7 @@ protected override void OnLoad(EventArgs e) private void SaveWhenIdle(object o, EventArgs eventArgs) { Application.Idle -= SaveWhenIdle; // don't need to do again till next Deactivate. - _model.SaveNow(); + _model.RequestBrowserToSave(); // Restore any tool state removed by CleanHtmlAndCopyToPageDom(), which is called by _model.SaveNow(). RunJavascriptAsync( "if (typeof(editTabBundle) !=='undefined') {editTabBundle.getToolboxBundleExports().applyToolboxStateToPage();}" @@ -2121,7 +2089,7 @@ internal void SetZoomControl(ZoomControl zoomCtl) private void _bookSettingsButton_Click(object sender, EventArgs e) { - _model.SaveNow(); + _model.RequestBrowserToSave(); // Open the book settings dialog to the context-specific group. var groupIndex = _model.CurrentPage.IsCoverPage ? 0 : 1; diff --git a/src/BloomExe/Edit/WebThumbNailList.cs b/src/BloomExe/Edit/WebThumbNailList.cs index 178fcc3f639e..5426c938cbd9 100644 --- a/src/BloomExe/Edit/WebThumbNailList.cs +++ b/src/BloomExe/Edit/WebThumbNailList.cs @@ -348,7 +348,7 @@ internal void PageMoved(IPage movedPage, int newPageIndex) WebSocketServer.SendString("pageThumbnailList", "pageListNeedsReset", ""); return; } - Model.SaveNow(); + Model.RequestBrowserToSave(); var relocatePageInfo = new RelocatePageInfo(movedPage, newPageIndex); RelocatePageEvent.Raise(relocatePageInfo); UpdateItems(movedPage.Book.GetPages()); diff --git a/src/BloomExe/Workspace/WorkspaceView.cs b/src/BloomExe/Workspace/WorkspaceView.cs index 79d6b2d762be..c056f5cd9236 100644 --- a/src/BloomExe/Workspace/WorkspaceView.cs +++ b/src/BloomExe/Workspace/WorkspaceView.cs @@ -1458,7 +1458,7 @@ private void StartProblemReport(object sender, EventArgs e) { if (_editTab.IsSelected) { - _editingView.Model.SaveNow(); + _editingView.Model.RequestBrowserToSave(); } } catch diff --git a/src/BloomExe/web/controllers/CopyrightAndLicenseApi.cs b/src/BloomExe/web/controllers/CopyrightAndLicenseApi.cs index 2b788e4ab523..578c95dfd72c 100644 --- a/src/BloomExe/web/controllers/CopyrightAndLicenseApi.cs +++ b/src/BloomExe/web/controllers/CopyrightAndLicenseApi.cs @@ -68,7 +68,7 @@ private void HandleBookCopyrightAndLicense(ApiRequest request) { case HttpMethods.Get: //in case we were in this dialog already and made changes which haven't found their way out to the book yet - Model.SaveNow(); + Model.RequestBrowserToSave(); var intellectualPropertyData = GetJsonFromMetadata( Model.CurrentBook.GetLicenseMetadata(), @@ -115,7 +115,7 @@ private void HandleImageCopyrightAndLicense(ApiRequest request) request.ReplyWithJson(intellectualPropertyData); break; case HttpMethods.Post: - View.Model.SaveNow(); // Saved DOM must be up to date with possibly new imageUrl + View.Model.RequestBrowserToSave(); // Saved DOM must be up to date with possibly new imageUrl metadata = GetMetadataFromJson(request, forBook: false); bool askUserToCopyToAllImages = View.SaveImageMetadata(metadata); diff --git a/src/BloomExe/web/controllers/EditingViewApi.cs b/src/BloomExe/web/controllers/EditingViewApi.cs index f0a3267fcd04..5d7af9e46198 100644 --- a/src/BloomExe/web/controllers/EditingViewApi.cs +++ b/src/BloomExe/web/controllers/EditingViewApi.cs @@ -67,6 +67,13 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler) HandleDuplicatePageMany, true ); + apiHandler.RegisterEndpointHandler( + "editView/saveHtml", + HandleSaveHtml, + true /*review*/ + , + true /*review*/ + ); apiHandler.RegisterEndpointHandler("editView/topics", HandleTopics, false); apiHandler.RegisterEndpointHandler("editView/changeImage", HandleChangeImage, true); apiHandler.RegisterEndpointHandler("editView/cutImage", HandleCutImage, true); @@ -231,6 +238,18 @@ dynamic messageBundle } } + private void HandleSaveHtml(ApiRequest request) + { + // the post is an object of the form {html: string, userStyles:string} + // after we get it we call the SaveHtmlNow method on the model + dynamic data = request.RequiredPostDynamic(); + var forceFullSave = (bool)data.forceFullSave; + var html = (string)data.html; + var userStylesheetContent = (string)data.userStylesheetContent; + View.Model.SavePageHtml(html, userStylesheetContent, forceFullSave); + request.PostSucceeded(); + } + private void HandleChangeImage(ApiRequest request) { dynamic data = DynamicJson.Parse(request.RequiredPostJson()); diff --git a/src/BloomExe/web/controllers/SignLanguageApi.cs b/src/BloomExe/web/controllers/SignLanguageApi.cs index 5fdaa1d03db2..0df2bd29bdee 100644 --- a/src/BloomExe/web/controllers/SignLanguageApi.cs +++ b/src/BloomExe/web/controllers/SignLanguageApi.cs @@ -572,7 +572,7 @@ public void CheckForChangedVideoOnActivate(object sender, EventArgs eventArgs) // We might modify the current page, but the user may also have modified it // without doing anything to cause a Save before the deactivate. So save their // changes before we go to work on it. - Model.SaveNow(); + Model.RequestBrowserToSave(); foreach (var videoPath in filesModifiedSinceDeactivate) { From 319d5e73a177ad317403261ddffe8f14e7474e7c Mon Sep 17 00:00:00 2001 From: Hatton Date: Tue, 30 Apr 2024 09:28:15 -0600 Subject: [PATCH 3/5] remove dangerous call to "pageSelectionChanging()" --- src/BloomBrowserUI/bookEdit/editablePage.ts | 5 +---- src/BloomBrowserUI/bookEdit/js/bloomEditing.ts | 7 +++---- src/BloomExe/Edit/EditingModel.cs | 4 +--- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/editablePage.ts b/src/BloomBrowserUI/bookEdit/editablePage.ts index 8d07a383dd9f..0728241f7c4e 100644 --- a/src/BloomBrowserUI/bookEdit/editablePage.ts +++ b/src/BloomBrowserUI/bookEdit/editablePage.ts @@ -11,7 +11,6 @@ import { theOneBubbleManager, BubbleManager } from "./js/bubbleManager"; // This allows strong typing to be done for exported functions export interface IPageFrameExports { - pageSelectionChanging(): void; pageUnloading(): void; disconnectForGarbageCollection(): void; copySelection(): void; @@ -38,9 +37,8 @@ export interface IPageFrameExports { } // This exports the functions that should be accessible from other IFrames or from C#. -// For example, editTabBundle.getEditablePageBundleExports().pageSelectionChanging() can be called. +// For example, editTabBundle.getEditablePageBundleExports().saveRequested() can be called. import { - pageSelectionChanging, saveRequested, userStylesheetContent, pageUnloading, @@ -53,7 +51,6 @@ import { attachToCkEditor } from "./js/bloomEditing"; export { - pageSelectionChanging, saveRequested, userStylesheetContent, pageUnloading, diff --git a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts index dc014c60b3e2..d6ba123ba1aa 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts @@ -1251,9 +1251,7 @@ export function localizeCkeditorTooltips(bar: JQuery) { }); } -// This is invoked from C# when we are about to change pages. The C# code will save the changes -// to the page after we return from this (hopefully; certainly in debugging this is the case). -export const pageSelectionChanging = () => { +function removeOrigami() { // We are mirroring the origami layoutToggleClickHandler() here, in case the user changes // pages while the origami toggle in on. // The DOM here is for just one page, so there's only ever one marginBox. @@ -1263,9 +1261,10 @@ export const pageSelectionChanging = () => { for (let i = 0; i < textLabels.length; i++) { textLabels[i].remove(); } -}; +} export function saveRequested(forceFullSave: boolean) { + removeOrigami(); postJson("editView/saveHtml", { forceFullSave: forceFullSave, html: getBodyContentForSavePage(), diff --git a/src/BloomExe/Edit/EditingModel.cs b/src/BloomExe/Edit/EditingModel.cs index 7ccbfdac88cb..809f92338e35 100644 --- a/src/BloomExe/Edit/EditingModel.cs +++ b/src/BloomExe/Edit/EditingModel.cs @@ -753,9 +753,6 @@ private void OnPageSelectionChangingInternal(object sender, EventArgs eventArgs) ) { _view.ChangingPages = true; - _view.RunJavascriptWithStringResult_Sync_Dangerous( - "if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getEditablePageBundleExports()) !=='undefined') {editTabBundle.getEditablePageBundleExports().pageSelectionChanging();}" - ); FinishSavingPage(); _view.RunJavascriptAsync( "if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getEditablePageBundleExports()) !=='undefined') {editTabBundle.getEditablePageBundleExports().disconnectForGarbageCollection();}" @@ -1294,6 +1291,7 @@ public void RequestBrowserToSave(bool forceFullSave = false) if (_domForCurrentPage != null && !_inProcessOfSaving && !NavigatingSoSuspendSaving) { Logger.WriteMinorEvent("EditingModel.RequestSave() starting"); + // show the saving message to the user _webSocketServer.SendString("pageThumbnailList", "saving", ""); _tasksToDoAfterSaving.Clear(); // review // review do we really need to be checking to see if things are loaded? If they are not, then there is nothing to save, and this doesn't thow. From 239b77880aa86d95ec9678ad8d00f86e391c5537 Mon Sep 17 00:00:00 2001 From: Hatton Date: Tue, 30 Apr 2024 09:28:48 -0600 Subject: [PATCH 4/5] remove long-unused methods in IBrowser --- src/BloomExe/IBrowser.cs | 124 --------------------------------------- 1 file changed, 124 deletions(-) diff --git a/src/BloomExe/IBrowser.cs b/src/BloomExe/IBrowser.cs index ff69b96e68f5..6f6503b80303 100644 --- a/src/BloomExe/IBrowser.cs +++ b/src/BloomExe/IBrowser.cs @@ -1,10 +1,7 @@ using Bloom.Api; using Bloom.Book; -using Bloom.ErrorReporter; using Bloom.ToPalaso; -using L10NSharp; using SIL.IO; -using SIL.Reporting; using System; using System.Diagnostics; using System.Drawing; @@ -288,127 +285,6 @@ public void OnOpenPageInEdge(object sender, EventArgs e) // intentionally letting any errors just escape, give us an error } - public void ReadEditedHtmlNow(string bodyHtml, string userCssContent) - { - if (Url != "about:blank") - { - LoadPageDomFromHtml(bodyHtml, userCssContent); - } - } - - /// - /// What's going on here: the browser is just editing/displaying a copy of one page of the document. - /// So we need to copy any changes back to the real DOM. - /// We're now obtaining the new content another way, so this code doesn't have any reason - /// to be in this class...but we're aiming for a minimal change, maximal safety fix for 4.9 - /// - private void LoadPageDomFromHtml(string bodyHtml, string userCssContent) - { - Debug.Assert(!InvokeRequired); - if (_pageEditDom == null) - return; - - try - { - // unlikely, but if we somehow couldn't get the new content, better keep the old. - // This MIGHT be able to happen in some cases of very fast page clicking, where - // the page isn't fully enough loaded to expose the functions we use to get the - // content. In that case, the user can't have made changes, so not saving is fine. - if (string.IsNullOrEmpty(bodyHtml)) - return; - - var content = bodyHtml; - XmlDocument dom; - - //todo: deal with exception that can come out of this - dom = XmlHtmlConverter.GetXmlDomFromHtml(content, false); - var bodyDom = dom.SelectSingleNode("//body"); - - if (_pageEditDom == null) - return; - - var destinationDomPage = _pageEditDom.SelectSingleNode( - "//body//div[contains(@class,'bloom-page')]" - ); - if (destinationDomPage == null) - return; - var expectedPageId = destinationDomPage.Attributes["id"].Value; - - var browserDomPage = bodyDom.SelectSingleNode( - "//body//div[contains(@class,'bloom-page')]" - ); - if (browserDomPage == null) - return; //why? but I've seen it happen - - var thisPageId = browserDomPage.Attributes["id"].Value; - if (expectedPageId != thisPageId) - { - SIL.Reporting.ErrorReport.NotifyUserOfProblem( - LocalizationManager.GetString( - "Browser.ProblemSaving", - "There was a problem while saving. Please return to the previous page and make sure it looks correct." - ) - ); - return; - } - - // We've seen pages get emptied out, and we don't know why. This is a safety check. - // See BL-13078, BL-13120, BL-13123, and BL-13143 for examples. - if (BookStorage.CheckForEmptyMarginBoxOnPage(browserDomPage as XmlElement)) - { - // This has been logged and reported to the user. We don't want to save the empty page. - return; - } - - _pageEditDom.GetElementsByTagName("body")[0].InnerXml = bodyDom.InnerXml; - - SaveCustomizedCssRules(userCssContent); - - //enhance: we have jscript for this: cleanup()... but running jscript in this method was leading the browser to show blank screen - // foreach (XmlElement j in _editDom.SafeSelectNodes("//div[contains(@class, 'ui-tooltip')]")) - // { - // j.ParentNode.RemoveChild(j); - // } - // foreach (XmlAttribute j in _editDom.SafeSelectNodes("//@ariasecondary-describedby | //@aria-describedby")) - // { - // j.OwnerElement.RemoveAttributeNode(j); - // } - } - catch (Exception e) - { - Bloom.Utils.MiscUtils.SuppressUnusedExceptionVarWarning(e); - Debug.Fail( - "Debug Mode Only: Error while trying to read changes to CSSRules. In Release, this just gets swallowed. Will now re-throw the exception." - ); -#if DEBUG - throw; -#endif - } - - try - { - XmlHtmlConverter.ThrowIfHtmlHasErrors(_pageEditDom.OuterXml); - } - catch (Exception e) - { - //var exceptionWithHtmlContents = new Exception(content); - ErrorReport.NotifyUserOfProblem( - e, - "Sorry, Bloom choked on something on this page (validating page).{1}{1}+{0}", - e.Message, - Environment.NewLine - ); - } - } - - private void SaveCustomizedCssRules(string userCssContent) - { - // Yes, this wipes out everything else in the head. At this point, the only things - // we need in _pageEditDom are the user defined style sheet and the bloom-page element in the body. - _pageEditDom.GetElementsByTagName("head")[0].InnerXml = - HtmlDom.CreateUserModifiedStyles(userCssContent); - } - [Obsolete( "This method is dangerous because it has to loop Application.DoEvents(). RunJavaScriptAsync() is preferred." )] From b74c29ad9fba665dd227d273a2bba890ca1ea956 Mon Sep 17 00:00:00 2001 From: Hatton Date: Tue, 30 Apr 2024 10:01:20 -0600 Subject: [PATCH 5/5] renaming and updating comments --- src/BloomExe/Edit/EditingModel.cs | 13 ++++--------- src/BloomExe/Edit/PageSelection.cs | 3 +++ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/BloomExe/Edit/EditingModel.cs b/src/BloomExe/Edit/EditingModel.cs index 809f92338e35..29eccc9ae2e3 100644 --- a/src/BloomExe/Edit/EditingModel.cs +++ b/src/BloomExe/Edit/EditingModel.cs @@ -753,7 +753,7 @@ private void OnPageSelectionChangingInternal(object sender, EventArgs eventArgs) ) { _view.ChangingPages = true; - FinishSavingPage(); + RequestSavePage(); _view.RunJavascriptAsync( "if (typeof(editTabBundle) !=='undefined' && typeof(editTabBundle.getEditablePageBundleExports()) !=='undefined') {editTabBundle.getEditablePageBundleExports().disconnectForGarbageCollection();}" ); @@ -1210,19 +1210,14 @@ internal void RethinkPageAndReloadIt(bool forceFullSave = false) { if (CannotSavePage()) return; - FinishSavingPage(forceFullSave); + RequestSavePage(forceFullSave); RefreshDisplayOfCurrentPage(); } /// - /// Called from a JavaScript event after it has done everything appropriate in JS land towards saving a page, - /// in the process of wrapping up this page before moving to another. - /// The main point is that any changes on this page get saved back to the main document. - /// In case it is an origami page, there is some special stuff to do as commented below. - /// (Argument is required for JS callback, not used). + /// Called as a result of page selection changing or other event that requests a save and reload. /// - /// true if it was aborted (nothing to save or refresh) - private void FinishSavingPage(bool forceFullSave = false) + private void RequestSavePage(bool forceFullSave = false) { if (CannotSavePage()) return; diff --git a/src/BloomExe/Edit/PageSelection.cs b/src/BloomExe/Edit/PageSelection.cs index 8b976030fae7..0d839d718a2a 100644 --- a/src/BloomExe/Edit/PageSelection.cs +++ b/src/BloomExe/Edit/PageSelection.cs @@ -28,7 +28,10 @@ public bool SelectPage(IPage page, bool prepareAlreadyDone = false) #endif //enhance... make pre-change event cancellable if (!prepareAlreadyDone) + { + // this leads to the saving of the current page PrepareToSelectPage(); + } _currentSelection = page; InvokeSelectionChanged();