{"action":"closed","issue":{"url":"https://api.github.com/repos/bootstrapworld/codemirror-blocks/issues/120","repository_url":"https://api.github.com/repos/bootstrapworld/codemirror-blocks","labels_url":"https://api.github.com/repos/bootstrapworld/codemirror-blocks/issues/120/labels{/name}","comments_url":"https://api.github.com/repos/bootstrapworld/codemirror-blocks/issues/120/comments","events_url":"https://api.github.com/repos/bootstrapworld/codemirror-blocks/issues/120/events","html_url":"https://github.com/bootstrapworld/codemirror-blocks/issues/120","id":341762402,"node_id":"MDU6SXNzdWUzNDE3NjI0MDI=","number":120,"title":"Tests that rely on events dispatched to quarantine nodes fail","user":{"login":"justinpombrio","id":434946,"node_id":"MDQ6VXNlcjQzNDk0Ng==","avatar_url":"https://avatars3.githubusercontent.com/u/434946?v=4","gravatar_id":"","url":"https://api.github.com/users/justinpombrio","html_url":"https://github.com/justinpombrio","followers_url":"https://api.github.com/users/justinpombrio/followers","following_url":"https://api.github.com/users/justinpombrio/following{/other_user}","gists_url":"https://api.github.com/users/justinpombrio/gists{/gist_id}","starred_url":"https://api.github.com/users/justinpombrio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/justinpombrio/subscriptions","organizations_url":"https://api.github.com/users/justinpombrio/orgs","repos_url":"https://api.github.com/users/justinpombrio/repos","events_url":"https://api.github.com/users/justinpombrio/events{/privacy}","received_events_url":"https://api.github.com/users/justinpombrio/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2018-07-17T04:32:39Z","updated_at":"2018-08-02T16:46:08Z","closed_at":"2018-08-02T16:46:08Z","author_association":"COLLABORATOR","body":"On my computer, this test case:\r\n\r\n    it('should save a valid, edited node on blur', function(done) {\r\n      this.literal.el.dispatchEvent(dblclick());\r\n      setTimeout(() => {\r\n        let selection = window.getSelection();\r\n        expect(selection.rangeCount).toEqual(1);\r\n        let range = selection.getRangeAt(0);\r\n        range.deleteContents();\r\n        range.insertNode(document.createTextNode('9'));\r\n        expect(this.cm.getValue()).toEqual('11');\r\n        document.activeElement.dispatchEvent(blur());\r\n        expect(this.cm.getValue()).toEqual('9');\r\n        expect(this.blocks.hasInvalidEdit).toBe(false); // This is true\r\n        done();\r\n      }, DELAY);\r\n    });\r\n\r\nfails with the following error message:\r\n\r\n    Chrome 64.0.3282 (Linux 0.0.0) The CodeMirrorBlocks Class events, should save a valid, edited node on blur FAILED\r\n    \tExpected true to be false.\r\n    \t    at <Jasmine>\r\n    \t    at webpack:///spec/justin.js:105:43 <- spec/index.js:25:97579\r\n    Chrome 64.0.3282 (Linux 0.0.0): Executed 1 of 1 (1 FAILED) ERROR (0.149 secs / 0.796 secs)\r\n    npm ERR! Test failed.  See above for more details.\r\n\r\nHere's what happens:\r\n\r\n1. The test case triggers a `blur` event, via this line:\r\n   `document.activeElement.dispatchEvent(blur());`\r\n2. The `onblur` handler activates and calls the `saveEdit` function in\r\n   `blocks.js`. So far so good.\r\n3. The `saveEdit` function gets down to this line:\r\n   `nodeEl.parentNode.removeChild(nodeEl);`\r\n4. Which, as it's executing but before removing the child, triggers a\r\n   `blur` event.\r\n5. The `onblur` handler activates again and calls `saveEdit`, which\r\n   carries forth, dutifully executes the\r\n   `nodeEl.parentNode.removeChild(nodeEl)` line as well as the rest of\r\n   the function, and returns.\r\n6. This second `onblur` event has now successfully completed, and\r\n   removed the child.\r\n7. The first `onblur` event tries to actually remove the child and\r\n   fails to, as it is no longer a child of that parent, throwing an\r\n   error and causing the test case to fail.\r\n\r\nThere's a [SO post discussing this surprising behavior](https://stackoverflow.com/questions/11600301/removing-an-element-through-the-parentnode-removechild-throws-a-dom-exception-8). I'm not sure why this would happen only for me.\r\n\r\nSee also the [MDN page on `.removeChild`](https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild), which mentions this behavior.\r\n\r\nLikewise, these two test cases appear to fail for me for a similarish reason:\r\n\r\n        it('should save whiteSpace on blur', function(done) {\r\n          this.whiteSpaceEl.dispatchEvent(dblclick());\r\n          setTimeout(() => {\r\n            // A blur event happens around here, but BEFORE the next line.\r\n            let quarantine = document.querySelectorAll('.blocks-editing')[0];\r\n            // quarantine is undefined\r\n            let selection = window.getSelection();\r\n            expect(selection.rangeCount).toEqual(1);\r\n            let range = selection.getRangeAt(0);\r\n            range.deleteContents();\r\n            range.insertNode(document.createTextNode('4253'));\r\n            // This line fails, because `quarantine` is undefined\r\n            quarantine.dispatchEvent(blur());\r\n            expect(this.cm.getValue()).toBe('(+ 1 4253 2) (+)');\r\n            expect(this.blocks.hasInvalidEdit).toBe(false);\r\n            done();\r\n          }, DELAY);\r\n        });\r\n\r\n        it('should not save anything & set all error state', function() {\r\n          // A blur event happens around here\r\n          let quarantine = document.querySelectorAll('.blocks-editing')[0];\r\n          expect(this.cm.replaceRange).not.toHaveBeenCalled();\r\n          expect(quarantine.classList).toContain('blocks-error');\r\n          expect(quarantine.title).toBe('Error: parse error');\r\n          expect(this.blocks.hasInvalidEdit).toBe(true);\r\n        });\r\n\r\nThey seem to fail due to stray `blur` events. As far as I can tell\r\nthey're coming from `handlers.default.call(this, node, event);` in the\r\n`nodeEventHandler` wrapper around calls to `insertionQuarantine` when\r\nhandling a double click."}}
{"actor":{"display_login":"schanzer"}}