🐛 Fixed a bug causing new drafts to only save if the title is populat…

…ed (TryGhost#20769) ref https://linear.app/tryghost/issue/ONC-253/drafts-only-save-if-the-title-is-populated - A [commit](TryGhost@c8ba9e8) in `v5.89.1` introduced a bug that caused new drafts to only save if the post title was populated, causing potential data loss if a user is working on a new draft without setting the title. - This commit reverts the one that introduced this bug to prevent data loss. This reverts commit c8ba9e8.
traPtitech · Aug 16, 2024 · 93cbb94 · 93cbb94
1 parent 663e787
commit 93cbb94
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 143 deletions.
diff --git a/ghost/admin/app/components/gh-koenig-editor-lexical.hbs b/ghost/admin/app/components/gh-koenig-editor-lexical.hbs
@@ -95,9 +95,7 @@
             @placeholder={{@bodyPlaceholder}}
             @cardConfig={{@cardOptions}}
             @onChange={{@onBodyChange}}
-            @updateSecondaryInstanceModel={{@updateSecondaryInstanceModel}}
             @registerAPI={{this.registerEditorAPI}}
-            @registerSecondaryAPI={{this.registerSecondaryEditorAPI}}
             @cursorDidExitAtTop={{if this.feature.editorExcerpt this.focusExcerpt this.focusTitle}}
             @updateWordCount={{@updateWordCount}}
             @updatePostTkCount={{@updatePostTkCount}}

diff --git a/ghost/admin/app/components/gh-koenig-editor-lexical.js b/ghost/admin/app/components/gh-koenig-editor-lexical.js
@@ -15,7 +15,6 @@ export default class GhKoenigEditorLexical extends Component {
     uploadUrl = `${ghostPaths().apiRoot}/images/upload/`;
 
     editorAPI = null;
-    secondaryEditorAPI = null;
     skipFocusEditor = false;
 
     @tracked titleIsHovered = false;
@@ -233,12 +232,6 @@ export default class GhKoenigEditorLexical extends Component {
         this.args.registerAPI(API);
     }
 
-    @action
-    registerSecondaryEditorAPI(API) {
-        this.secondaryEditorAPI = API;
-        this.args.registerSecondaryAPI(API);
-    }
-
     // focus the editor when the editor canvas is clicked below the editor content,
     // otherwise the browser will defocus the editor and the cursor will disappear
     @action

diff --git a/ghost/admin/app/components/gh-post-settings-menu.hbs b/ghost/admin/app/components/gh-post-settings-menu.hbs
@@ -853,7 +853,6 @@
                     post=this.post
                     editorAPI=this.editorAPI
                     toggleSettingsMenu=this.toggleSettingsMenu
-                    secondaryEditorAPI=this.secondaryEditorAPI
                 }}
                 @close={{this.closePostHistory}}
                 @modifier="total-overlay post-history" />

diff --git a/ghost/admin/app/components/koenig-lexical-editor.js b/ghost/admin/app/components/koenig-lexical-editor.js
@@ -678,43 +678,34 @@ export default class KoenigLexicalEditor extends Component {
         const multiplayerDocId = cardConfig.post.id;
         const multiplayerUsername = this.session.user.name;
 
-        const KGEditorComponent = ({isInitInstance}) => {
-            return (
-                <div data-secondary-instance={isInitInstance ? true : false} style={isInitInstance ? {width: 0, height: 0, overflow: 'hidden'} : {}}>
-                    <KoenigComposer
-                        editorResource={this.editorResource}
-                        cardConfig={cardConfig}
-                        enableMultiplayer={enableMultiplayer}
-                        fileUploader={{useFileUpload, fileTypes}}
-                        initialEditorState={this.args.lexical}
-                        multiplayerUsername={multiplayerUsername}
-                        multiplayerDocId={multiplayerDocId}
-                        multiplayerEndpoint={multiplayerEndpoint}
-                        onError={this.onError}
-                        darkMode={this.feature.nightShift}
-                        isTKEnabled={true}
-                    >
-                        <KoenigEditor
-                            editorResource={this.editorResource}
-                            cursorDidExitAtTop={isInitInstance ? null : this.args.cursorDidExitAtTop}
-                            placeholderText={isInitInstance ? null : this.args.placeholderText}
-                            darkMode={isInitInstance ? null : this.feature.nightShift}
-                            onChange={isInitInstance ? this.args.updateSecondaryInstanceModel : this.args.onChange}
-                            registerAPI={isInitInstance ? this.args.registerSecondaryAPI : this.args.registerAPI}
-                        />
-                        <WordCountPlugin editorResource={this.editorResource} onChange={isInitInstance ? () => {} : this.args.updateWordCount} />
-                        <TKCountPlugin editorResource={this.editorResource} onChange={isInitInstance ? () => {} : this.args.updatePostTkCount} />
-                    </KoenigComposer>
-                </div>
-            );
-        };
-
         return (
             <div className={['koenig-react-editor', 'koenig-lexical', this.args.className].filter(Boolean).join(' ')}>
                 <ErrorHandler config={this.config}>
                     <Suspense fallback={<p className="koenig-react-editor-loading">Loading editor...</p>}>
-                        <KGEditorComponent />
-                        <KGEditorComponent isInitInstance={true} />
+                        <KoenigComposer
+                            editorResource={this.editorResource}
+                            cardConfig={cardConfig}
+                            enableMultiplayer={enableMultiplayer}
+                            fileUploader={{useFileUpload, fileTypes}}
+                            initialEditorState={this.args.lexical}
+                            multiplayerUsername={multiplayerUsername}
+                            multiplayerDocId={multiplayerDocId}
+                            multiplayerEndpoint={multiplayerEndpoint}
+                            onError={this.onError}
+                            darkMode={this.feature.nightShift}
+                            isTKEnabled={true}
+                        >
+                            <KoenigEditor
+                                editorResource={this.editorResource}
+                                cursorDidExitAtTop={this.args.cursorDidExitAtTop}
+                                placeholderText={this.args.placeholder}
+                                darkMode={this.feature.nightShift}
+                                onChange={this.args.onChange}
+                                registerAPI={this.args.registerAPI}
+                            />
+                            <WordCountPlugin editorResource={this.editorResource} onChange={this.args.updateWordCount} />
+                            <TKCountPlugin editorResource={this.editorResource} onChange={this.args.updatePostTkCount} />
+                        </KoenigComposer>
                     </Suspense>
                 </ErrorHandler>
             </div>

diff --git a/ghost/admin/app/components/modal-post-history.hbs b/ghost/admin/app/components/modal-post-history.hbs
@@ -33,7 +33,6 @@
                     @lexical={{this.selectedRevision.lexical}}
                     @cardConfig={{this.cardConfig}}
                     @registerAPI={{this.registerSelectedEditorApi}}
-                    @registerSecondaryAPI={{this.registerSecondarySelectedEditorApi}}
                 />
             </div>
         </div>

diff --git a/ghost/admin/app/components/modal-post-history.js b/ghost/admin/app/components/modal-post-history.js
@@ -31,7 +31,6 @@ export default class ModalPostHistory extends Component {
         super(...arguments);
         this.post = this.args.model.post;
         this.editorAPI = this.args.model.editorAPI;
-        this.secondaryEditorAPI = this.args.model.secondaryEditorAPI;
         this.toggleSettingsMenu = this.args.model.toggleSettingsMenu;
     }
 
@@ -102,11 +101,6 @@ export default class ModalPostHistory extends Component {
         this.selectedEditor = api;
     }
 
-    @action
-    registerSecondarySelectedEditorApi(api) {
-        this.secondarySelectedEditor = api;
-    }
-
     @action
     registerComparisonEditorApi(api) {
         this.comparisonEditor = api;
@@ -136,7 +130,6 @@ export default class ModalPostHistory extends Component {
             updateEditor: () => {
                 const state = this.editorAPI.editorInstance.parseEditorState(revision.lexical);
                 this.editorAPI.editorInstance.setEditorState(state);
-                this.secondaryEditorAPI.editorInstance.setEditorState(state);
             },
             closePostHistoryModal: () => {
                 this.closeModal();

diff --git a/ghost/admin/app/controllers/lexical-editor.js b/ghost/admin/app/controllers/lexical-editor.js
@@ -297,11 +297,6 @@ export default class LexicalEditorController extends Controller {
         this._timedSaveTask.perform();
     }
 
-    @action
-    updateSecondaryInstanceModel(lexical) {
-        this.set('post.secondaryLexicalState', JSON.stringify(lexical));
-    }
-
     @action
     updateTitleScratch(title) {
         this.set('post.titleScratch', title);
@@ -428,11 +423,6 @@ export default class LexicalEditorController extends Controller {
         this.editorAPI = API;
     }
 
-    @action
-    registerSecondaryEditorAPI(API) {
-        this.secondaryEditorAPI = API;
-    }
-
     @action
     clearFeatureImage() {
         this.post.set('featureImage', null);
@@ -1231,14 +1221,16 @@ export default class LexicalEditorController extends Controller {
         _timedSaveTask;
 
     /* Private methods -------------------------------------------------------*/
+
     _hasDirtyAttributes() {
         let post = this.post;
 
         if (!post) {
             return false;
         }
 
-        // If the Adapter failed to save the post, isError will be true, and we should consider the post still dirty.
+        // if the Adapter failed to save the post isError will be true
+        // and we should consider the post still dirty.
         if (post.get('isError')) {
             this._leaveModalReason = {reason: 'isError', context: post.errors.messages};
             return true;
@@ -1253,32 +1245,37 @@ export default class LexicalEditorController extends Controller {
             return true;
         }
 
-        // Title scratch comparison
+        // titleScratch isn't an attr so needs a manual dirty check
         if (post.titleScratch !== post.title) {
             this._leaveModalReason = {reason: 'title is different', context: {current: post.title, scratch: post.titleScratch}};
             return true;
         }
 
-        // Lexical and scratch comparison
+        // scratch isn't an attr so needs a manual dirty check
         let lexical = post.get('lexical');
         let scratch = post.get('lexicalScratch');
-        let secondaryLexical = post.get('secondaryLexicalState');
-
-        let lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : [];
-        let scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : [];
-        let secondaryLexicalChildNodes = secondaryLexical ? JSON.parse(secondaryLexical).root?.children : [];
-
-        lexicalChildNodes.forEach(child => child.direction = null);
-        scratchChildNodes.forEach(child => child.direction = null);
-        secondaryLexicalChildNodes.forEach(child => child.direction = null);
-
-        // Compare initLexical with scratch
-        let isSecondaryDirty = secondaryLexical && scratch && JSON.stringify(secondaryLexicalChildNodes) !== JSON.stringify(scratchChildNodes);
+        // additional guard in case we are trying to compare null with undefined
+        if (scratch || lexical) {
+            if (scratch !== lexical) {
+                // lexical can dynamically set direction on loading editor state (e.g. "rtl"/"ltr") per the DOM context
+                //  and we need to ignore this as a change from the user; see https://github.com/facebook/lexical/issues/4998
+                const scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : [];
+                const lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : [];
+
+                // // nullling is typically faster than delete
+                scratchChildNodes.forEach(child => child.direction = null);
+                lexicalChildNodes.forEach(child => child.direction = null);
+
+                if (JSON.stringify(scratchChildNodes) === JSON.stringify(lexicalChildNodes)) {
+                    return false;
+                }
 
-        // Compare lexical with scratch
-        let isLexicalDirty = lexical && scratch && JSON.stringify(lexicalChildNodes) !== JSON.stringify(scratchChildNodes);
+                this._leaveModalReason = {reason: 'lexical is different', context: {current: lexical, scratch}};
+                return true;
+            }
+        }
 
-        // New+unsaved posts always return `hasDirtyAttributes: true`
+        // new+unsaved posts always return `hasDirtyAttributes: true`
         // so we need a manual check to see if any
         if (post.get('isNew')) {
             let changedAttributes = Object.keys(post.changedAttributes());
@@ -1289,26 +1286,15 @@ export default class LexicalEditorController extends Controller {
             return changedAttributes.length ? true : false;
         }
 
-        // We've covered all the non-tracked cases we care about so fall
+        // we've covered all the non-tracked cases we care about so fall
         // back on Ember Data's default dirty attribute checks
         let {hasDirtyAttributes} = post;
+
         if (hasDirtyAttributes) {
             this._leaveModalReason = {reason: 'post.hasDirtyAttributes === true', context: post.changedAttributes()};
-            return true;
         }
 
-        // If either comparison is not dirty, return false, because scratch is always up to date.
-        if (!isSecondaryDirty || !isLexicalDirty) {
-            return false;
-        }
-
-        // If both comparisons are dirty, consider the post dirty
-        if (isSecondaryDirty && isLexicalDirty) {
-            this._leaveModalReason = {reason: 'initLexical and lexical are different from scratch', context: {secondaryLexical, lexical, scratch}};
-            return true;
-        }
-
-        return false;
+        return hasDirtyAttributes;
     }
 
     _showSaveNotification(prevStatus, status, delayed) {

diff --git a/ghost/admin/app/models/post.js b/ghost/admin/app/models/post.js
@@ -136,9 +136,6 @@ export default Model.extend(Comparable, ValidationEngine, {
     scratch: null,
     lexicalScratch: null,
     titleScratch: null,
-    //This is used to store the initial lexical state from the
-    // secondary editor to get the schema up to date in case its outdated
-    secondaryLexicalState: null,
 
     // For use by date/time pickers - will be validated then converted to UTC
     // on save. Updated by an observer whenever publishedAtUTC changes.

diff --git a/ghost/admin/app/templates/lexical-editor.hbs b/ghost/admin/app/templates/lexical-editor.hbs
@@ -73,7 +73,6 @@
                 @body={{readonly this.post.lexicalScratch}}
                 @bodyPlaceholder={{concat "Begin writing your " this.post.displayName "..."}}
                 @onBodyChange={{this.updateScratch}}
-                @updateSecondaryInstanceModel={{this.updateSecondaryInstanceModel}}
                 @headerOffset={{editor.headerHeight}}
                 @scrollContainerSelector=".gh-koenig-editor"
                 @scrollOffsetBottomSelector=".gh-mobile-nav-bar"
@@ -98,7 +97,6 @@
                 }}
                 @postType={{this.post.displayName}}
                 @registerAPI={{this.registerEditorAPI}}
-                @registerSecondaryAPI={{this.registerSecondaryEditorAPI}}
                 @savePostTask={{this.savePostTask}}
             />
 
@@ -138,7 +136,6 @@
                 @updateSlugTask={{this.updateSlugTask}}
                 @savePostTask={{this.savePostTask}}
                 @editorAPI={{this.editorAPI}}
-                @secondaryEditorAPI={{this.secondaryEditorAPI}}
                 @toggleSettingsMenu={{this.toggleSettingsMenu}}
             />
         {{/if}}

diff --git a/ghost/admin/tests/unit/controllers/editor-test.js b/ghost/admin/tests/unit/controllers/editor-test.js
@@ -208,8 +208,7 @@ describe('Unit: Controller: lexical-editor', function () {
                 titleScratch: 'this is a title',
                 status: 'published',
                 lexical: initialLexicalString,
-                lexicalScratch: initialLexicalString,
-                secondaryLexicalState: initialLexicalString
+                lexicalScratch: initialLexicalString
             }));
 
             // synthetically update the lexicalScratch as if the editor itself made the modifications on loading the initial editorState
@@ -226,47 +225,5 @@ describe('Unit: Controller: lexical-editor', function () {
             isDirty = controller.get('hasDirtyAttributes');
             expect(isDirty).to.be.true;
         });
-
-        it('dirty is false if secondaryLexical and scratch matches, but lexical is outdated', async function () {
-            const initialLexicalString = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": null,"format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-            const lexicalScratch = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-            const secondLexicalInstance = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Here's some new text","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-
-            let controller = this.owner.lookup('controller:lexical-editor');
-            controller.set('post', EmberObject.create({
-                title: 'this is a title',
-                titleScratch: 'this is a title',
-                status: 'published',
-                lexical: initialLexicalString,
-                lexicalScratch: lexicalScratch,
-                secondaryLexicalState: secondLexicalInstance
-            }));
-
-            let isDirty = controller.get('hasDirtyAttributes');
-
-            expect(isDirty).to.be.false;
-        });
-
-        it('dirty is true if secondaryLexical and lexical does not match scratch', async function () {
-            const initialLexicalString = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": null,"format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-            const lexicalScratch = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content1234","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-            const secondLexicalInstance = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Here's some new text","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
-
-            let controller = this.owner.lookup('controller:lexical-editor');
-            controller.set('post', EmberObject.create({
-                title: 'this is a title',
-                titleScratch: 'this is a title',
-                status: 'published',
-                lexical: initialLexicalString,
-                lexicalScratch: lexicalScratch,
-                secondaryLexicalState: secondLexicalInstance
-            }));
-
-            controller.send('updateScratch',JSON.parse(lexicalScratch));
-
-            let isDirty = controller.get('hasDirtyAttributes');
-
-            expect(isDirty).to.be.true;
-        });
     });
 });