From ff608ab524c182d95d09b0f9c3618450f8f6b103 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 8 Aug 2017 15:32:24 -0400 Subject: [PATCH 01/38] :arrow_up: github --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 06710e579..4db9c61ab 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "exception-reporting": "0.41.4", "find-and-replace": "0.209.5", "fuzzy-finder": "1.5.8", - "github": "0.4.0", + "github": "0.4.1", "git-diff": "1.3.6", "go-to-line": "0.32.1", "grammar-selector": "0.49.5", From 276fcb9e9693538bf55bdc8ebacde15b9f0484c1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 8 Aug 2017 16:52:38 -0400 Subject: [PATCH 02/38] :arrow_up: electron-link --- script/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/package.json b/script/package.json index 52f0b6d55..454d561aa 100644 --- a/script/package.json +++ b/script/package.json @@ -9,7 +9,7 @@ "csslint": "1.0.2", "donna": "1.0.16", "electron-chromedriver": "~1.6", - "electron-link": "0.1.0", + "electron-link": "0.1.1", "electron-mksnapshot": "~1.6", "electron-packager": "7.3.0", "electron-winstaller": "2.6.2", From 35a7ae6d2e3fd6766211c6fdc63cba0c1dbef937 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 8 Aug 2017 17:36:23 -0400 Subject: [PATCH 03/38] :arrow_up: github --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4db9c61ab..8e3981529 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "exception-reporting": "0.41.4", "find-and-replace": "0.209.5", "fuzzy-finder": "1.5.8", - "github": "0.4.1", + "github": "0.4.2", "git-diff": "1.3.6", "go-to-line": "0.32.1", "grammar-selector": "0.49.5", From 98296bf95027f2a9668e9d078ffcf1ce25cf1c88 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Tue, 8 Aug 2017 21:29:22 -0400 Subject: [PATCH 04/38] :arrow_up: language-java@0.27.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 181cbfa12..79dda7061 100644 --- a/package.json +++ b/package.json @@ -145,7 +145,7 @@ "language-go": "0.44.2", "language-html": "0.47.3", "language-hyperlink": "0.16.2", - "language-java": "0.27.2", + "language-java": "0.27.3", "language-javascript": "0.127.1", "language-json": "0.19.1", "language-less": "0.33.0", From 9fe67290c5abf3e2aaefebe356133f7741e2ac33 Mon Sep 17 00:00:00 2001 From: Nontawat Numor Date: Wed, 9 Aug 2017 12:16:19 +0700 Subject: [PATCH 05/38] Update document with electron version :tea: --- docs/native-profiling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/native-profiling.md b/docs/native-profiling.md index 58a164982..afac6b4ab 100644 --- a/docs/native-profiling.md +++ b/docs/native-profiling.md @@ -6,7 +6,7 @@ * Open the dev tools with `alt-cmd-i` * Evaluate `process.versions.electron` in the console. * Based on this version, download the appropriate Electron symbols from the [releases](https://github.com/atom/electron/releases) page. - * The file name should look like `electron-v0.X.Y-darwin-x64-dsym.zip`. + * The file name should look like `electron-v1.X.Y-darwin-x64-dsym.zip`. * Decompress these symbols in your `~/Downloads` directory. * Now create a time profile in Instruments. * Open `Instruments.app`. From a09b634e1bc56c37056f8c949c6ab18116aec3c4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 09:18:41 -0400 Subject: [PATCH 06/38] Separate tests and installer creation on AppVeyor --- appveyor.yml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 6a8d3ac91..4ed91e643 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -19,9 +19,13 @@ environment: global: ATOM_DEV_RESOURCE_PATH: c:\projects\atom TEST_JUNIT_XML_ROOT: c:\projects\junit-test-results + NODE_VERSION: 6.9.4 matrix: - - NODE_VERSION: 6.9.4 + - TASK: test + BUILD_ARG: + - TASK: installer + BUILD_ARG: --create-windows-installer matrix: fast_finish: true @@ -34,14 +38,16 @@ install: build_script: - CD %APPVEYOR_BUILD_FOLDER% - - script\build.cmd --code-sign --compress-artifacts + - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% test_script: - - script\lint.cmd - - script\test.cmd - -after_test: - - IF [%APPVEYOR_REPO_BRANCH:~-9%]==[-releases] ( script\create-installer.cmd ) + - | + IF [%TASK]==[test] ( + script\lint.cmd + script\test.cmd + ) ELSE ( + ECHO + ) deploy: off artifacts: From 1233bbccb00be78387a1cbcfe412cc84ba8a967b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 09:20:52 -0400 Subject: [PATCH 07/38] Missing % --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 4ed91e643..ae5f97d7b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -42,7 +42,7 @@ build_script: test_script: - | - IF [%TASK]==[test] ( + IF [%TASK%]==[test] ( script\lint.cmd script\test.cmd ) ELSE ( From 2d186820cf05ea74f200eebea056ccc9be35d0b0 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 09:28:37 -0400 Subject: [PATCH 08/38] The matrix will be slightly prettier this way <_< >_> --- appveyor.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index ae5f97d7b..c366f7afa 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -23,9 +23,7 @@ environment: matrix: - TASK: test - BUILD_ARG: - TASK: installer - BUILD_ARG: --create-windows-installer matrix: fast_finish: true @@ -38,6 +36,12 @@ install: build_script: - CD %APPVEYOR_BUILD_FOLDER% + - | + IF [%TASK%]==[installer] ( + SET BUILD_ARG=--create-windows-installer + ) ELSE ( + SET BUILD_ARG= + ) - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% test_script: From 3a9872e8091d5dfc9cb832feee10628b939395c8 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 10:02:24 -0400 Subject: [PATCH 09/38] YAML syntax tweaks --- appveyor.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index c366f7afa..7bf8be73e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -36,8 +36,7 @@ install: build_script: - CD %APPVEYOR_BUILD_FOLDER% - - | - IF [%TASK%]==[installer] ( + - IF [%TASK%]==[installer] ( SET BUILD_ARG=--create-windows-installer ) ELSE ( SET BUILD_ARG= @@ -45,12 +44,11 @@ build_script: - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% test_script: - - | - IF [%TASK%]==[test] ( - script\lint.cmd + - IF [%TASK%]==[test] ( + script\lint.cmd && script\test.cmd ) ELSE ( - ECHO + ECHO Skipping tests ) deploy: off From fbd4b688dd611b1f828f3ea0135e8d7737bfa76d Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Wed, 9 Aug 2017 11:17:48 -0400 Subject: [PATCH 10/38] :arrow_up: language-javascript@0.127.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 79dda7061..6f07b00d9 100644 --- a/package.json +++ b/package.json @@ -146,7 +146,7 @@ "language-html": "0.47.3", "language-hyperlink": "0.16.2", "language-java": "0.27.3", - "language-javascript": "0.127.1", + "language-javascript": "0.127.2", "language-json": "0.19.1", "language-less": "0.33.0", "language-make": "0.22.3", From 4acd52a978011d8d84df8fa25e0bb533da6a19b4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 12:18:07 -0400 Subject: [PATCH 11/38] Skip installer build on non-release branches --- appveyor.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 7bf8be73e..f0880c10b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -37,9 +37,14 @@ install: build_script: - CD %APPVEYOR_BUILD_FOLDER% - IF [%TASK%]==[installer] ( - SET BUILD_ARG=--create-windows-installer + IF [%APPVEYOR_REPO_BRANCH:~-9%]==[-releases] ( + SET BUILD_ARG=--create-windows-installer + ) ELSE ( + ECHO "Skipping installer build on non-release branch" + ) ) ELSE ( - SET BUILD_ARG= + SET BUILD_ARG= && + ECHO "Skipping installer build on non-installer build matrix row" ) - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% From 2088143a9050feb2ece9f70deea2e2cb431f8864 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Wed, 9 Aug 2017 11:49:21 -0400 Subject: [PATCH 12/38] Fix specs --- spec/tokenized-buffer-spec.coffee | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/tokenized-buffer-spec.coffee b/spec/tokenized-buffer-spec.coffee index 5b1863b35..07e7e80e6 100644 --- a/spec/tokenized-buffer-spec.coffee +++ b/spec/tokenized-buffer-spec.coffee @@ -184,7 +184,7 @@ describe "TokenizedBuffer", -> it "schedules the invalidated lines to be tokenized in the background", -> buffer.insert([5, 30], '/* */') buffer.setTextInRange([[2, 0], [3, 0]], '/*') - expect(tokenizedBuffer.tokenizedLines[2].tokens[0].scopes).toEqual ['source.js', 'comment.block.js', 'punctuation.definition.comment.js'] + expect(tokenizedBuffer.tokenizedLines[2].tokens[0].scopes).toEqual ['source.js', 'comment.block.js', 'punctuation.definition.comment.begin.js'] expect(tokenizedBuffer.tokenizedLines[3].tokens[0].scopes).toEqual ['source.js'] advanceClock() @@ -214,7 +214,7 @@ describe "TokenizedBuffer", -> it "schedules the invalidated lines to be tokenized in the background", -> buffer.insert([5, 30], '/* */') buffer.insert([2, 0], '/*\nabcde\nabcder') - expect(tokenizedBuffer.tokenizedLines[2].tokens[0].scopes).toEqual ['source.js', 'comment.block.js', 'punctuation.definition.comment.js'] + expect(tokenizedBuffer.tokenizedLines[2].tokens[0].scopes).toEqual ['source.js', 'comment.block.js', 'punctuation.definition.comment.begin.js'] expect(tokenizedBuffer.tokenizedLines[3].tokens[0].scopes).toEqual ['source.js', 'comment.block.js'] expect(tokenizedBuffer.tokenizedLines[4].tokens[0].scopes).toEqual ['source.js', 'comment.block.js'] expect(tokenizedBuffer.tokenizedLines[5].tokens[0].scopes).toEqual ['source.js'] @@ -596,10 +596,10 @@ describe "TokenizedBuffer", -> {position: Point(0, 9), closeTags: ["syntax--keyword syntax--operator syntax--assignment syntax--js"], openTags: []} {position: Point(0, 10), closeTags: [], openTags: ["syntax--constant syntax--numeric syntax--decimal syntax--js"]} {position: Point(0, 11), closeTags: ["syntax--constant syntax--numeric syntax--decimal syntax--js"], openTags: []} - {position: Point(0, 12), closeTags: [], openTags: ["syntax--comment syntax--block syntax--js", "syntax--punctuation syntax--definition syntax--comment syntax--js"]} - {position: Point(0, 14), closeTags: ["syntax--punctuation syntax--definition syntax--comment syntax--js"], openTags: []} - {position: Point(1, 5), closeTags: [], openTags: ["syntax--punctuation syntax--definition syntax--comment syntax--js"]} - {position: Point(1, 7), closeTags: ["syntax--punctuation syntax--definition syntax--comment syntax--js", "syntax--comment syntax--block syntax--js"], openTags: ["syntax--storage syntax--type syntax--var syntax--js"]} + {position: Point(0, 12), closeTags: [], openTags: ["syntax--comment syntax--block syntax--js", "syntax--punctuation syntax--definition syntax--comment syntax--begin syntax--js"]} + {position: Point(0, 14), closeTags: ["syntax--punctuation syntax--definition syntax--comment syntax--begin syntax--js"], openTags: []} + {position: Point(1, 5), closeTags: [], openTags: ["syntax--punctuation syntax--definition syntax--comment syntax--end syntax--js"]} + {position: Point(1, 7), closeTags: ["syntax--punctuation syntax--definition syntax--comment syntax--end syntax--js", "syntax--comment syntax--block syntax--js"], openTags: ["syntax--storage syntax--type syntax--var syntax--js"]} {position: Point(1, 10), closeTags: ["syntax--storage syntax--type syntax--var syntax--js"], openTags: []} {position: Point(1, 15), closeTags: [], openTags: ["syntax--keyword syntax--operator syntax--assignment syntax--js"]} {position: Point(1, 16), closeTags: ["syntax--keyword syntax--operator syntax--assignment syntax--js"], openTags: []} From 0db8c9c3ece5932dab5ca9e533d3a474905d5df2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 13:01:40 -0400 Subject: [PATCH 13/38] Don't build on non-release installer rows either --- appveyor.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index f0880c10b..0f566f936 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -38,15 +38,20 @@ build_script: - CD %APPVEYOR_BUILD_FOLDER% - IF [%TASK%]==[installer] ( IF [%APPVEYOR_REPO_BRANCH:~-9%]==[-releases] ( - SET BUILD_ARG=--create-windows-installer + SET BUILD_ARG=--create-windows-installer && + SET BUILD=yes ) ELSE ( - ECHO "Skipping installer build on non-release branch" + ECHO "Skipping installer and Atom build on non-release branch" && + SET BUILD=no ) ) ELSE ( SET BUILD_ARG= && + SET BUILD=yes && ECHO "Skipping installer build on non-installer build matrix row" ) - - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% + - IF [%BUILD]==[yes] ( + script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% + ) test_script: - IF [%TASK%]==[test] ( From ee12c1c7d2c032da13048852cdd6654460a9591f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 13:10:26 -0400 Subject: [PATCH 14/38] Why does my brain always edit out the second % --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 0f566f936..8df08bc01 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -49,7 +49,7 @@ build_script: SET BUILD=yes && ECHO "Skipping installer build on non-installer build matrix row" ) - - IF [%BUILD]==[yes] ( + - IF [%BUILD%]==[yes] ( script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% ) From ee0df014bc95167282f1e3317ffc5e138bffac77 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 9 Aug 2017 13:15:34 -0400 Subject: [PATCH 15/38] Less variable magic --- appveyor.yml | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 8df08bc01..ee95a5d5e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -38,19 +38,13 @@ build_script: - CD %APPVEYOR_BUILD_FOLDER% - IF [%TASK%]==[installer] ( IF [%APPVEYOR_REPO_BRANCH:~-9%]==[-releases] ( - SET BUILD_ARG=--create-windows-installer && - SET BUILD=yes + script\build.cmd --code-sign --compress-artifacts --create-windows-installer ) ELSE ( - ECHO "Skipping installer and Atom build on non-release branch" && - SET BUILD=no + ECHO Skipping installer and Atom build on non-release branch ) ) ELSE ( - SET BUILD_ARG= && - SET BUILD=yes && - ECHO "Skipping installer build on non-installer build matrix row" - ) - - IF [%BUILD%]==[yes] ( - script\build.cmd --code-sign --compress-artifacts %BUILD_ARG% + ECHO Skipping installer build on non-installer build matrix row && + script\build.cmd --code-sign --compress-artifacts ) test_script: @@ -58,7 +52,7 @@ test_script: script\lint.cmd && script\test.cmd ) ELSE ( - ECHO Skipping tests + ECHO Skipping tests on installer build matrix row ) deploy: off From 2fa2feacafa95a82e363e8b036f2ad95f08d5227 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Wed, 9 Aug 2017 22:52:39 -0400 Subject: [PATCH 16/38] Multiline is important, don't forget to set it --- src/workspace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/workspace.js b/src/workspace.js index 3bf112461..ad37630eb 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1949,7 +1949,7 @@ module.exports = class Workspace extends Model { } if (!outOfProcessFinished.length) { - let flags = 'g' + let flags = 'gm' // set multiline flag so ^ and $ match start/end of line, not file if (regex.ignoreCase) { flags += 'i' } const task = Task.once( From 95b216f2345589aa36ea6801babb27bd6526a226 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Wed, 9 Aug 2017 23:15:33 -0400 Subject: [PATCH 17/38] Add multiline spec --- spec/workspace-spec.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index bdd5677c8..add555f28 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -2394,6 +2394,22 @@ i = /test/; #FIXME\ expect(results[0].replacements).toBe(6) }) }) + + it('uses the multiline flag when searching', () => { + const filePath = path.join(projectDir, 'sample.js') + fs.copyFileSync(path.join(fixturesDir, 'sample.js'), filePath) + + const results = [] + waitsForPromise(() => + atom.workspace.replace(/;$/gi, 'items', [filePath], result => results.push(result)) + ) + + runs(() => { + expect(results).toHaveLength(1) + expect(results[0].filePath).toBe(filePath) + expect(results[0].replacements).toBe(8) + }) + }) }) describe('when a buffer is already open', () => { From 9fb4f0d9cdee00b9f21ffb44f308cbc8bb660a5b Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Wed, 9 Aug 2017 16:04:49 -0700 Subject: [PATCH 18/38] Create shorter temp path for Squirrel --- appveyor.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/appveyor.yml b/appveyor.yml index ee95a5d5e..5195d81cc 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -36,6 +36,8 @@ install: build_script: - CD %APPVEYOR_BUILD_FOLDER% + - IF NOT EXIST C:\tmp MKDIR C:\tmp + - SET SQUIRREL_TEMP=C:\tmp - IF [%TASK%]==[installer] ( IF [%APPVEYOR_REPO_BRANCH:~-9%]==[-releases] ( script\build.cmd --code-sign --compress-artifacts --create-windows-installer From 00d27befe8719b4ab40041a353dfe62c148c559b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 10 Aug 2017 17:25:50 +0200 Subject: [PATCH 19/38] Create, update and destroy highlights manually Etch's reconciliation routine causes elements to be sometimes re-ordered. In order to move an element, however, Etch needs to first detach it from the DOM and then re-append it at the right location. This behavior is unacceptable for highlight decorations because it could re-start CSS animations on a certain highlight decoration when a completely different one is added or removed. Even though we are still interested in restructuring etch's reconciliation logic to prevent unwanted re-orderings, with this commit we are switching to a custom routine to create/update/remove highlight decorations that prevents unnecessary moves and, as a result, fixes the undesired behavior described above. --- spec/text-editor-component-spec.js | 21 +++++++ src/text-editor-component.js | 88 +++++++++++++++++++++--------- 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 3b2f2072a..2c13b48af 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -1524,6 +1524,27 @@ describe('TextEditorComponent', () => { await setScrollTop(component, component.getLineHeight() * 3) expect(element.querySelectorAll('.highlight.a').length).toBe(0) }) + + it('does not move existing highlights when adding or removing other highlight decorations (regression)', async () => { + const {component, element, editor} = buildComponent() + + const marker1 = editor.markScreenRange([[1, 6], [1, 10]]) + editor.decorateMarker(marker1, {type: 'highlight', class: 'a'}) + await component.getNextUpdatePromise() + const marker1Region = element.querySelector('.highlight.a') + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + + const marker2 = editor.markScreenRange([[1, 2], [1, 4]]) + editor.decorateMarker(marker2, {type: 'highlight', class: 'b'}) + await component.getNextUpdatePromise() + const marker2Region = element.querySelector('.highlight.b') + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + expect(Array.from(marker2Region.parentElement.children).indexOf(marker2Region)).toBe(1) + + marker2.destroy() + await component.getNextUpdatePromise() + expect(Array.from(marker1Region.parentElement.children).indexOf(marker1Region)).toBe(0) + }) }) describe('overlay decorations', () => { diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 881aaad81..970ee29c1 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -3417,8 +3417,10 @@ class CursorsAndInputComponent { class LinesTileComponent { constructor (props) { + this.highlightComponentsByKey = new Map() this.props = props etch.initialize(this) + this.updateHighlights() this.createLines() this.updateBlockDecorations({}, props) } @@ -3432,13 +3434,22 @@ class LinesTileComponent { this.updateLines(oldProps, newProps) this.updateBlockDecorations(oldProps, newProps) } + this.updateHighlights() } } destroy () { + this.highlightComponentsByKey.forEach((highlightComponent) => { + highlightComponent.destroy() + }) + this.highlightComponentsByKey.clear() + for (let i = 0; i < this.lineComponents.length; i++) { this.lineComponents[i].destroy() } + this.lineComponents.length = 0 + + return etch.destroy(this) } render () { @@ -3456,34 +3467,12 @@ class LinesTileComponent { backgroundColor: 'inherit' } }, - this.renderHighlights() - // Lines and block decorations will be manually inserted here for efficiency - ) - } - - renderHighlights () { - const {top, lineHeight, highlightDecorations} = this.props - - let children = null - if (highlightDecorations) { - const decorationCount = highlightDecorations.length - children = new Array(decorationCount) - for (let i = 0; i < decorationCount; i++) { - const highlightProps = Object.assign( - {parentTileTop: top, lineHeight}, - highlightDecorations[i] - ) - children[i] = $(HighlightComponent, highlightProps) - highlightDecorations[i].flashRequested = false - } - } - - return $.div( - { + $.div({ + ref: 'highlights', className: 'highlights', - style: {contain: 'layout'} - }, - children + style: {layout: 'contain'} + }) + // Lines and block decorations will be manually inserted here for efficiency ) } @@ -3676,6 +3665,40 @@ class LinesTileComponent { } } + updateHighlights () { + const {top, lineHeight, highlightDecorations} = this.props + + const visibleHighlightDecorations = new Set() + if (highlightDecorations) { + for (let i = 0; i < highlightDecorations.length; i++) { + const highlightDecoration = highlightDecorations[i] + + const highlightProps = Object.assign( + {parentTileTop: top, lineHeight}, + highlightDecorations[i] + ) + let highlightComponent = this.highlightComponentsByKey.get(highlightDecoration.key) + if (highlightComponent) { + highlightComponent.update(highlightProps) + } else { + highlightComponent = new HighlightComponent(highlightProps) + this.refs.highlights.appendChild(highlightComponent.element) + this.highlightComponentsByKey.set(highlightDecoration.key, highlightComponent) + } + + highlightDecorations[i].flashRequested = false + visibleHighlightDecorations.add(highlightDecoration.key) + } + } + + this.highlightComponentsByKey.forEach((highlightComponent, key) => { + if (!visibleHighlightDecorations.has(key)) { + highlightComponent.destroy() + this.highlightComponentsByKey.delete(key) + } + }) + } + shouldUpdate (newProps) { const oldProps = this.props if (oldProps.top !== newProps.top) return true @@ -3876,6 +3899,17 @@ class HighlightComponent { if (this.props.flashRequested) this.performFlash() } + destroy () { + if (this.timeoutsByClassName) { + this.timeoutsByClassName.forEach((timeout) => { + window.clearTimeout(timeout) + }) + this.timeoutsByClassName.clear() + } + + return etch.destroy(this) + } + update (newProps) { this.props = newProps etch.updateSync(this) From 8963cf495517bddf197441f54deb112167d8b209 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Thu, 10 Aug 2017 13:24:46 -0400 Subject: [PATCH 20/38] Only use multiline if the flag is passed in --- spec/workspace-spec.js | 4 ++-- src/workspace.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index add555f28..476a4ba5b 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -2395,13 +2395,13 @@ i = /test/; #FIXME\ }) }) - it('uses the multiline flag when searching', () => { + it('does not discard the multiline flag', () => { const filePath = path.join(projectDir, 'sample.js') fs.copyFileSync(path.join(fixturesDir, 'sample.js'), filePath) const results = [] waitsForPromise(() => - atom.workspace.replace(/;$/gi, 'items', [filePath], result => results.push(result)) + atom.workspace.replace(/;$/gmi, 'items', [filePath], result => results.push(result)) ) runs(() => { diff --git a/src/workspace.js b/src/workspace.js index ad37630eb..17c6b2a8b 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1949,7 +1949,8 @@ module.exports = class Workspace extends Model { } if (!outOfProcessFinished.length) { - let flags = 'gm' // set multiline flag so ^ and $ match start/end of line, not file + let flags = 'g' + if (regex.multiline) { flags += 'm' } if (regex.ignoreCase) { flags += 'i' } const task = Task.once( From 7f8f184e969478189b1d6f759cf17fcb76da6779 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 10 Aug 2017 12:55:41 -0600 Subject: [PATCH 21/38] Shim rowsPerPage property on Editor instances Several packages were relying on a raw property rather than the getter method. This isn't really supported, but may as well keep them working. --- src/text-editor.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/text-editor.coffee b/src/text-editor.coffee index 6962bf10a..39abd05a0 100644 --- a/src/text-editor.coffee +++ b/src/text-editor.coffee @@ -3527,6 +3527,10 @@ class TextEditor extends Model else 1 + Object.defineProperty(@prototype, 'rowsPerPage', { + get: -> @getRowsPerPage() + }) + ### Section: Config ### From b14c4a32c98fffbdcd7859b7536bc2b07e5b6926 Mon Sep 17 00:00:00 2001 From: Linus Eriksson Date: Fri, 11 Aug 2017 21:26:31 +0200 Subject: [PATCH 22/38] :arrow_up: bracket-matcher@0.87.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6f07b00d9..8bcb93f7e 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "autosave": "0.24.3", "background-tips": "0.27.1", "bookmarks": "0.44.4", - "bracket-matcher": "0.87.0", + "bracket-matcher": "0.87.3", "command-palette": "0.40.4", "dalek": "0.2.1", "deprecation-cop": "0.56.7", From 5a8b197db19f9328063f0aa8aaecc725eb620be8 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Fri, 11 Aug 2017 13:24:34 -0700 Subject: [PATCH 23/38] Ensure Pane.destroyItem always returns a promise Fixes #15157 --- spec/pane-spec.js | 25 ++++++++++++++++--------- src/pane.coffee | 5 ++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/spec/pane-spec.js b/spec/pane-spec.js index c36abbf6a..68e93c38f 100644 --- a/spec/pane-spec.js +++ b/spec/pane-spec.js @@ -551,10 +551,11 @@ describe('Pane', () => { itemURI = 'test' confirm.andReturn(0) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) @@ -565,11 +566,12 @@ describe('Pane', () => { showSaveDialog.andReturn('/selected/path') confirm.andReturn(0) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(showSaveDialog).toHaveBeenCalled() expect(item1.saveAs).toHaveBeenCalledWith('/selected/path') expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) @@ -578,10 +580,11 @@ describe('Pane', () => { it('removes and destroys the item without saving it', async () => { confirm.andReturn(2) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true); }) }) @@ -589,19 +592,21 @@ describe('Pane', () => { it('does not save, remove, or destroy the item', async () => { confirm.andReturn(1) - await pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(true) expect(item1.isDestroyed()).toBe(false) + expect(success).toBe(false) }) }) describe('when force=true', () => { it('destroys the item immediately', async () => { - await pane.destroyItem(item1, true) + const success = await pane.destroyItem(item1, true) expect(item1.save).not.toHaveBeenCalled() expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) @@ -630,18 +635,20 @@ describe('Pane', () => { }) describe('when passed a permanent dock item', () => { - it("doesn't destroy the item", () => { + it("doesn't destroy the item", async () => { spyOn(item1, 'isPermanentDockItem').andReturn(true) - pane.destroyItem(item1) + const success = await pane.destroyItem(item1) expect(pane.getItems().includes(item1)).toBe(true) expect(item1.isDestroyed()).toBe(false) + expect(success).toBe(false); }) - it('destroy the item if force=true', () => { + it('destroy the item if force=true', async () => { spyOn(item1, 'isPermanentDockItem').andReturn(true) - pane.destroyItem(item1, true) + const success = await pane.destroyItem(item1, true) expect(pane.getItems().includes(item1)).toBe(false) expect(item1.isDestroyed()).toBe(true) + expect(success).toBe(true) }) }) }) diff --git a/src/pane.coffee b/src/pane.coffee index 9c0440e0a..23a60306b 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -621,12 +621,15 @@ class Pane destroyItem: (item, force) -> index = @items.indexOf(item) if index isnt -1 - return false if not force and @getContainer()?.getLocation() isnt 'center' and item.isPermanentDockItem?() + if not force and @getContainer()?.getLocation() isnt 'center' and item.isPermanentDockItem?() + return Promise.resolve(false) + @emitter.emit 'will-destroy-item', {item, index} @container?.willDestroyPaneItem({item, index, pane: this}) if force or not item?.shouldPromptToSave?() @removeItem(item, false) item.destroy?() + Promise.resolve(true) else @promptToSaveItem(item).then (result) => if result From 8667cfdd13b1f28e01358e5399ffe19b0e1cbadd Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 11 Aug 2017 15:39:42 -0600 Subject: [PATCH 24/38] Work around incorrect data on compositionupdate events in Chrome 56 --- src/text-editor-component.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 970ee29c1..0c80ef52c 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1651,7 +1651,14 @@ class TextEditorComponent { } didCompositionUpdate (event) { - this.props.model.insertText(event.data, {select: true}) + if (parseInt(process.versions.chrome) === 56) { + process.nextTick(() => { + const previewText = this.refs.cursorsAndInput.refs.hiddenInput.value + this.props.model.insertText(previewText, {select: true}) + }) + } else { + this.props.model.insertText(event.data, {select: true}) + } } didCompositionEnd (event) { From 54a6f0d29faf886a24a50287cd386171054d324d Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 11 Aug 2017 15:57:46 -0600 Subject: [PATCH 25/38] Clear hidden input compositionstart on Chrome 56 We use the value of the hidden input to display a preview of the composition, but it might already contain spaces from previous keystrokes, since we don't call preventDefault when spaces are inserted. --- src/text-editor-component.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 0c80ef52c..79105f868 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1644,6 +1644,10 @@ class TextEditorComponent { // 4. compositionend fired // 5. textInput fired; event.data == the completion string didCompositionStart () { + if (parseInt(process.versions.chrome) === 56) { + this.getHiddenInput().value = '' + } + this.compositionCheckpoint = this.props.model.createCheckpoint() if (this.accentedCharacterMenuIsOpen) { this.props.model.selectLeft() @@ -1653,7 +1657,7 @@ class TextEditorComponent { didCompositionUpdate (event) { if (parseInt(process.versions.chrome) === 56) { process.nextTick(() => { - const previewText = this.refs.cursorsAndInput.refs.hiddenInput.value + const previewText = this.getHiddenInput().value this.props.model.insertText(previewText, {select: true}) }) } else { @@ -2815,6 +2819,10 @@ class TextEditorComponent { return this.props.inputEnabled != null ? this.props.inputEnabled : true } + getHiddenInput () { + return this.refs.cursorsAndInput.refs.hiddenInput + } + getPlatform () { return this.props.platform || process.platform } From 2e048826fb565c87725809a5d722049e57d1f7aa Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Fri, 11 Aug 2017 15:58:55 -0700 Subject: [PATCH 26/38] :arrow_up: apm --- apm/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm/package.json b/apm/package.json index ba8d06f1d..7117147f1 100644 --- a/apm/package.json +++ b/apm/package.json @@ -6,6 +6,6 @@ "url": "https://github.com/atom/atom.git" }, "dependencies": { - "atom-package-manager": "1.18.3" + "atom-package-manager": "1.18.4" } } From 3e0d79005012382569acfb1480cde79d47337032 Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Fri, 11 Aug 2017 17:01:34 -0700 Subject: [PATCH 27/38] Remove language-typescript deprecation. New package soon. --- script/deprecated-packages.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/script/deprecated-packages.json b/script/deprecated-packages.json index 12638967e..dc97e3734 100644 --- a/script/deprecated-packages.json +++ b/script/deprecated-packages.json @@ -875,10 +875,6 @@ "hasDeprecations": true, "latestHasDeprecations": false }, - "language-typescript": { - "hasAlternative": true, - "alternative": "atom-typescript" - }, "laravel-facades": { "version": "<=1.0.0", "hasDeprecations": true, From ca183dd6938dd2a1724f7cdc65e7f4ec91afaffe Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 10:06:56 +0200 Subject: [PATCH 28/38] Don't insert IME preview on next tick if composition has already ended --- src/text-editor-component.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 79105f868..a1867a931 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1657,8 +1657,10 @@ class TextEditorComponent { didCompositionUpdate (event) { if (parseInt(process.versions.chrome) === 56) { process.nextTick(() => { - const previewText = this.getHiddenInput().value - this.props.model.insertText(previewText, {select: true}) + if (this.compositionCheckpoint) { + const previewText = this.getHiddenInput().value + this.props.model.insertText(previewText, {select: true}) + } }) } else { this.props.model.insertText(event.data, {select: true}) From fc1327eb22265d2ae37db25cf9e9f8107e3b67fb Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 12:28:32 +0200 Subject: [PATCH 29/38] Fix measuring lines in presence of pending autoscroll requests Calling `pixelPositionForScreenPosition` was sometimes throwing an error indicating that the requested position was not rendered and that, as such, could not be measured. This was caused by trying to measure a line that was visible at the moment of the call while also having a pending autoscroll request that would cause that line to go off-screen. Due to how the code was structured, we would mistakenly detect that line as visible, autoscroll to a different location, re-render a different region of the buffer and then try to measure the now invisible line. This commit fixes this issue by restructuring and simplifying the logic for rendering extra lines in order to measure them. Now, every line for which a measurement has been requested is stored in a `linesToMeasure` map. During the first phase of the update process (after honoring autoscroll requests), we detect which of these lines are currently visible and if they're not, store them into the `extraRenderedScreenLines` map, which is then used to render lines that are invisible but need to be measured. --- spec/text-editor-component-spec.js | 19 ++++++++ src/text-editor-component.js | 78 +++++++++++++++--------------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 2c13b48af..9dcd2c32c 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -3399,6 +3399,15 @@ describe('TextEditorComponent', () => { expect(top).toBe(clientTopForLine(referenceComponent, 12) - referenceContentRect.top) expect(left).toBe(clientLeftForCharacter(referenceComponent, 12, 1) - referenceContentRect.left) } + + // Measuring a currently rendered line while an autoscroll that causes + // that line to go off-screen is in progress. + { + editor.setCursorScreenPosition([10, 0]) + const {top, left} = component.pixelPositionForScreenPosition({row: 3, column: 5}) + expect(top).toBe(clientTopForLine(referenceComponent, 3) - referenceContentRect.top) + expect(left).toBe(clientLeftForCharacter(referenceComponent, 3, 5) - referenceContentRect.left) + } }) it('does not get the component into an inconsistent state when the model has unflushed changes (regression)', async () => { @@ -3445,6 +3454,16 @@ describe('TextEditorComponent', () => { pixelPosition.left += component.getBaseCharacterWidth() / 3 expect(component.screenPositionForPixelPosition(pixelPosition)).toEqual([12, 1]) } + + // Measuring a currently rendered line while an autoscroll that causes + // that line to go off-screen is in progress. + { + const pixelPosition = referenceComponent.pixelPositionForScreenPosition({row: 3, column: 4}) + pixelPosition.top += component.getLineHeight() / 3 + pixelPosition.left += component.getBaseCharacterWidth() / 3 + editor.setCursorBufferPosition([10, 0]) + expect(component.screenPositionForPixelPosition(pixelPosition)).toEqual([3, 4]) + } }) }) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 970ee29c1..53e8d4882 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -110,8 +110,8 @@ class TextEditorComponent { this.cursorsBlinking = false this.cursorsBlinkedOff = false this.nextUpdateOnlyBlinksCursors = null - this.extraLinesToMeasure = null - this.extraRenderedScreenLines = null + this.linesToMeasure = new Map() + this.extraRenderedScreenLines = new Map() this.horizontalPositionsToMeasure = new Map() // Keys are rows with positions we want to measure, values are arrays of columns to measure this.horizontalPixelPositionsByScreenLineId = new Map() // Values are maps from column to horiontal pixel positions this.blockDecorationsToMeasure = new Set() @@ -355,6 +355,7 @@ class TextEditorComponent { this.queryLineNumbersToRender() this.queryGuttersToRender() this.queryDecorationsToRender() + this.queryExtraScreenLinesToRender() this.shouldRenderDummyScrollbars = !this.remeasureScrollbars etch.updateSync(this) this.updateClassList() @@ -369,8 +370,6 @@ class TextEditorComponent { } const wasHorizontalScrollbarVisible = this.isHorizontalScrollbarVisible() - this.extraRenderedScreenLines = this.extraLinesToMeasure - this.extraLinesToMeasure = null this.measureLongestLineWidth() this.measureHorizontalPositions() this.updateAbsolutePositionedDecorations() @@ -606,21 +605,19 @@ class TextEditorComponent { }) } - if (this.extraLinesToMeasure) { - this.extraLinesToMeasure.forEach((screenLine, screenRow) => { - if (screenRow < startRow || screenRow >= endRow) { - tileNodes.push($(LineComponent, { - key: 'extra-' + screenLine.id, - screenLine, - screenRow, - displayLayer, - nodePool: this.lineNodesPool, - lineNodesByScreenLineId, - textNodesByScreenLineId - })) - } - }) - } + this.extraRenderedScreenLines.forEach((screenLine, screenRow) => { + if (screenRow < startRow || screenRow >= endRow) { + tileNodes.push($(LineComponent, { + key: 'extra-' + screenLine.id, + screenLine, + screenRow, + displayLayer, + nodePool: this.lineNodesPool, + lineNodesByScreenLineId, + textNodesByScreenLineId + })) + } + }) return $.div({ key: 'lineTiles', @@ -830,12 +827,22 @@ class TextEditorComponent { const longestLineRow = model.getApproximateLongestScreenRow() const longestLine = model.screenLineForScreenRow(longestLineRow) if (longestLine !== this.previousLongestLine) { - this.requestExtraLineToMeasure(longestLineRow, longestLine) + this.requestLineToMeasure(longestLineRow, longestLine) this.longestLineToMeasure = longestLine this.previousLongestLine = longestLine } } + queryExtraScreenLinesToRender () { + this.extraRenderedScreenLines.clear() + this.linesToMeasure.forEach((screenLine, row) => { + if (row < this.getRenderedStartRow() || row >= this.getRenderedEndRow()) { + this.extraRenderedScreenLines.set(row, screenLine) + } + }) + this.linesToMeasure.clear() + } + queryLineNumbersToRender () { const {model} = this.props if (!model.isLineNumberGutterVisible()) return @@ -906,7 +913,7 @@ class TextEditorComponent { renderedScreenLineForRow (row) { return ( this.renderedScreenLines[row - this.getRenderedStartRow()] || - (this.extraRenderedScreenLines ? this.extraRenderedScreenLines.get(row) : null) + this.extraRenderedScreenLines.get(row) ) } @@ -2125,29 +2132,24 @@ class TextEditorComponent { } } - requestExtraLineToMeasure (row, screenLine) { - if (!this.extraLinesToMeasure) this.extraLinesToMeasure = new Map() - this.extraLinesToMeasure.set(row, screenLine) + requestLineToMeasure (row, screenLine) { + this.linesToMeasure.set(row, screenLine) } requestHorizontalMeasurement (row, column) { if (column === 0) return - if (row < this.getRenderedStartRow() || row >= this.getRenderedEndRow()) { - const screenLine = this.props.model.screenLineForScreenRow(row) - if (screenLine) { - this.requestExtraLineToMeasure(row, screenLine) - } else { - return - } - } + const screenLine = this.props.model.screenLineForScreenRow(row) + if (screenLine) { + this.requestLineToMeasure(row, screenLine) - let columns = this.horizontalPositionsToMeasure.get(row) - if (columns == null) { - columns = [] - this.horizontalPositionsToMeasure.set(row, columns) + let columns = this.horizontalPositionsToMeasure.get(row) + if (columns == null) { + columns = [] + this.horizontalPositionsToMeasure.set(row, columns) + } + columns.push(column) } - columns.push(column) } measureHorizontalPositions () { @@ -2260,7 +2262,7 @@ class TextEditorComponent { let screenLine = this.renderedScreenLineForRow(row) if (!screenLine) { - this.requestExtraLineToMeasure(row, model.screenLineForScreenRow(row)) + this.requestLineToMeasure(row, model.screenLineForScreenRow(row)) this.updateSyncBeforeMeasuringContent() this.measureContentDuringUpdateSync() screenLine = this.renderedScreenLineForRow(row) From b4f029e9f0885789f63df3ec24c58466f38c77e2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 10:11:14 +0200 Subject: [PATCH 30/38] Test both Chrome 56 and other Chrome versions IME behavior --- spec/text-editor-component-spec.js | 598 ++++++++++++++++++++--------- src/text-editor-component.js | 8 +- 2 files changed, 419 insertions(+), 187 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 2c13b48af..c52cb53a8 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -3049,198 +3049,421 @@ describe('TextEditorComponent', () => { }) describe('keyboard input', () => { - it('handles inserted accented characters via the press-and-hold menu on macOS correctly', () => { - const {editor, component, element} = buildComponent({text: ''}) - editor.insertText('x') - editor.setCursorBufferPosition([0, 1]) + describe('on Chrome 56', () => { + it('handles inserted accented characters via the press-and-hold menu on macOS correctly', async () => { + const {editor, component, element} = buildComponent({text: '', chromeVersion: 56}) + editor.insertText('x') + editor.setCursorBufferPosition([0, 1]) - // Simulate holding the A key to open the press-and-hold menu, - // then closing it via ESC. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didKeydown({code: 'Escape'}) - component.didKeyup({code: 'Escape'}) - expect(editor.getText()).toBe('xa') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xaa') - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate holding the A key to open the press-and-hold menu, + // then closing it via ESC. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'Escape'}) + component.didKeyup({code: 'Escape'}) + expect(editor.getText()).toBe('xa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xaa') + editor.undo() + expect(editor.getText()).toBe('x') - // Simulate holding the A key to open the press-and-hold menu, - // then selecting an alternative by typing a number. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didKeydown({code: 'Digit2'}) - component.didKeyup({code: 'Digit2'}) - component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) - expect(editor.getText()).toBe('xá') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xáa') - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate holding the A key to open the press-and-hold menu, + // then selecting an alternative by typing a number. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'Digit2'}) + component.didKeyup({code: 'Digit2'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') - // Simulate holding the A key to open the press-and-hold menu, - // then selecting an alternative by clicking on it. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) - expect(editor.getText()).toBe('xá') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xáa') - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate holding the A key to open the press-and-hold menu, + // then selecting an alternative by clicking on it. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') - // Simulate holding the A key to open the press-and-hold menu, - // cycling through the alternatives with the arrows, then selecting one of them with Enter. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionStart({data: ''}) - component.didCompositionUpdate({data: 'à'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xà') - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionUpdate({data: 'á'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xá') - component.didKeydown({code: 'Enter'}) - component.didCompositionUpdate({data: 'á'}) - component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) - component.didCompositionEnd({data: 'á', target: component.refs.cursorsAndInput.refs.hiddenInput}) - component.didKeyup({code: 'Enter'}) - expect(editor.getText()).toBe('xá') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xáa') - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then selecting one of them with Enter. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.getHiddenInput().value = 'à' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xá') + component.didKeydown({code: 'Enter'}) + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'á', target: component.getHiddenInput()}) + component.didKeyup({code: 'Enter'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xá') - // Simulate holding the A key to open the press-and-hold menu, - // cycling through the alternatives with the arrows, then closing it via ESC. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionStart({data: ''}) - component.didCompositionUpdate({data: 'à'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xà') - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionUpdate({data: 'á'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xá') - component.didKeydown({code: 'Escape'}) - component.didCompositionUpdate({data: 'a'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) - component.didKeyup({code: 'Escape'}) - expect(editor.getText()).toBe('xa') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xaa') - editor.undo() - expect(editor.getText()).toBe('x') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') - // Simulate pressing the O key and holding the A key to open the press-and-hold menu right before releasing the O key, - // cycling through the alternatives with the arrows, then closing it via ESC. - component.didKeydown({code: 'KeyO'}) - component.didKeypress({code: 'KeyO'}) - component.didTextInput({data: 'o', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyO'}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionStart({data: ''}) - component.didCompositionUpdate({data: 'à'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xoà') - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionUpdate({data: 'á'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xoá') - component.didKeydown({code: 'Escape'}) - component.didCompositionUpdate({data: 'a'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) - component.didKeyup({code: 'Escape'}) - expect(editor.getText()).toBe('xoa') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then closing it via ESC. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.getHiddenInput().value = 'à' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xá') + component.didKeydown({code: 'Escape'}) + component.didCompositionUpdate({data: 'a'}) + component.getHiddenInput().value = 'a' + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) + component.didKeyup({code: 'Escape'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xaa') + editor.undo() + expect(editor.getText()).toBe('x') - // Simulate holding the A key to open the press-and-hold menu, - // cycling through the alternatives with the arrows, then closing it by changing focus. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeydown({code: 'KeyA'}) - component.didKeydown({code: 'KeyA'}) - component.didKeyup({code: 'KeyA'}) - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionStart({data: ''}) - component.didCompositionUpdate({data: 'à'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xà') - component.didKeydown({code: 'ArrowRight'}) - component.didCompositionUpdate({data: 'á'}) - component.didKeyup({code: 'ArrowRight'}) - expect(editor.getText()).toBe('xá') - component.didCompositionUpdate({data: 'á'}) - component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) - component.didCompositionEnd({data: 'á', target: component.refs.cursorsAndInput.refs.hiddenInput}) - expect(editor.getText()).toBe('xá') - // Ensure another "a" can be typed correctly. - component.didKeydown({code: 'KeyA'}) - component.didKeypress({code: 'KeyA'}) - component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) - component.didKeyup({code: 'KeyA'}) - expect(editor.getText()).toBe('xáa') - editor.undo() - expect(editor.getText()).toBe('x') + // Simulate pressing the O key and holding the A key to open the press-and-hold menu right before releasing the O key, + // cycling through the alternatives with the arrows, then closing it via ESC. + component.didKeydown({code: 'KeyO'}) + component.didKeypress({code: 'KeyO'}) + component.didTextInput({data: 'o', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyO'}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.getHiddenInput().value = 'à' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xoà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xoá') + component.didKeydown({code: 'Escape'}) + component.didCompositionUpdate({data: 'a'}) + component.getHiddenInput().value = 'a' + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) + component.didKeyup({code: 'Escape'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xoa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then closing it by changing focus. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.getHiddenInput().value = 'à' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didKeyup({code: 'ArrowRight'}) + await getNextTickPromise() + expect(editor.getText()).toBe('xá') + component.didCompositionUpdate({data: 'á'}) + component.getHiddenInput().value = 'á' + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'á', target: component.refs.cursorsAndInput.refs.hiddenInput}) + await getNextTickPromise() + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') + }) + }) + + describe('on other versions of Chrome', () => { + it('handles inserted accented characters via the press-and-hold menu on macOS correctly', () => { + const {editor, component, element} = buildComponent({text: '', chromeVersion: 57}) + editor.insertText('x') + editor.setCursorBufferPosition([0, 1]) + + // Simulate holding the A key to open the press-and-hold menu, + // then closing it via ESC. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'Escape'}) + component.didKeyup({code: 'Escape'}) + expect(editor.getText()).toBe('xa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xaa') + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // then selecting an alternative by typing a number. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'Digit2'}) + component.didKeyup({code: 'Digit2'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // then selecting an alternative by clicking on it. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then selecting one of them with Enter. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xá') + component.didKeydown({code: 'Enter'}) + component.didCompositionUpdate({data: 'á'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'á', target: component.refs.cursorsAndInput.refs.hiddenInput}) + component.didKeyup({code: 'Enter'}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then closing it via ESC. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xá') + component.didKeydown({code: 'Escape'}) + component.didCompositionUpdate({data: 'a'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) + component.didKeyup({code: 'Escape'}) + expect(editor.getText()).toBe('xa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xaa') + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate pressing the O key and holding the A key to open the press-and-hold menu right before releasing the O key, + // cycling through the alternatives with the arrows, then closing it via ESC. + component.didKeydown({code: 'KeyO'}) + component.didKeypress({code: 'KeyO'}) + component.didTextInput({data: 'o', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyO'}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xoà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xoá') + component.didKeydown({code: 'Escape'}) + component.didCompositionUpdate({data: 'a'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'a', target: component.refs.cursorsAndInput.refs.hiddenInput}) + component.didKeyup({code: 'Escape'}) + expect(editor.getText()).toBe('xoa') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + editor.undo() + expect(editor.getText()).toBe('x') + + // Simulate holding the A key to open the press-and-hold menu, + // cycling through the alternatives with the arrows, then closing it by changing focus. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeydown({code: 'KeyA'}) + component.didKeydown({code: 'KeyA'}) + component.didKeyup({code: 'KeyA'}) + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionStart({data: ''}) + component.didCompositionUpdate({data: 'à'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xà') + component.didKeydown({code: 'ArrowRight'}) + component.didCompositionUpdate({data: 'á'}) + component.didKeyup({code: 'ArrowRight'}) + expect(editor.getText()).toBe('xá') + component.didCompositionUpdate({data: 'á'}) + component.didTextInput({data: 'á', stopPropagation: () => {}, preventDefault: () => {}}) + component.didCompositionEnd({data: 'á', target: component.refs.cursorsAndInput.refs.hiddenInput}) + expect(editor.getText()).toBe('xá') + // Ensure another "a" can be typed correctly. + component.didKeydown({code: 'KeyA'}) + component.didKeypress({code: 'KeyA'}) + component.didTextInput({data: 'a', stopPropagation: () => {}, preventDefault: () => {}}) + component.didKeyup({code: 'KeyA'}) + expect(editor.getText()).toBe('xáa') + editor.undo() + expect(editor.getText()).toBe('x') + }) }) }) @@ -3546,6 +3769,7 @@ function buildComponent (params = {}) { rowsPerTile: params.rowsPerTile, updatedSynchronously: params.updatedSynchronously || false, platform: params.platform, + chromeVersion: params.chromeVersion, mouseWheelScrollSensitivity: params.mouseWheelScrollSensitivity }) const {element} = component @@ -3679,3 +3903,7 @@ function getElementHeight (element) { bottomRuler.remove() return height } + +function getNextTickPromise () { + return new Promise((resolve) => process.nextTick(resolve)) +} diff --git a/src/text-editor-component.js b/src/text-editor-component.js index a1867a931..d315df360 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1644,7 +1644,7 @@ class TextEditorComponent { // 4. compositionend fired // 5. textInput fired; event.data == the completion string didCompositionStart () { - if (parseInt(process.versions.chrome) === 56) { + if (this.getChromeVersion() === 56) { this.getHiddenInput().value = '' } @@ -1655,7 +1655,7 @@ class TextEditorComponent { } didCompositionUpdate (event) { - if (parseInt(process.versions.chrome) === 56) { + if (this.getChromeVersion() === 56) { process.nextTick(() => { if (this.compositionCheckpoint) { const previewText = this.getHiddenInput().value @@ -2828,6 +2828,10 @@ class TextEditorComponent { getPlatform () { return this.props.platform || process.platform } + + getChromeVersion () { + return this.props.chromeVersion || parseInt(process.versions.chrome) + } } class DummyScrollbarComponent { From 964f209c404e5aa6e62dbbd7c0f70a92605b318f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 15:33:20 +0200 Subject: [PATCH 31/38] Don't throw an error when setting an incredibly small `lineHeight` Instead, if the measured line height equals 0, default it to 1 so that the editor component doesn't start computing `NaN` or `Infinity` values due to e.g. dividing by 0. We should probably consider sanitizing line heights smaller than a certain threshold, but that's non trivial because line height is expressed as a multiplier of the font size. Also, users may style the `line-height` property via CSS, which may still throw errors when using small values. --- spec/text-editor-component-spec.js | 38 ++++++++++++++++++++++++++++++ src/text-editor-component.js | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 2c13b48af..b2f94b097 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -119,6 +119,44 @@ describe('TextEditorComponent', () => { } }) + it('re-renders lines when their height changes', async () => { + const {component, element, editor} = buildComponent({rowsPerTile: 3, autoHeight: false}) + element.style.height = 4 * component.measurements.lineHeight + 'px' + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(9) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(9) + + element.style.lineHeight = '2.0' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(6) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(6) + + element.style.lineHeight = '0.7' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(12) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(12) + + element.style.lineHeight = '0.05' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(13) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(13) + + element.style.lineHeight = '0' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(13) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(13) + + element.style.lineHeight = '1' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + expect(element.querySelectorAll('.line-number:not(.dummy)').length).toBe(9) + expect(element.querySelectorAll('.line:not(.dummy)').length).toBe(9) + }) + it('makes the content at least as tall as the scroll container client height', async () => { const {component, element, editor} = buildComponent({text: 'a', height: 100}) expect(component.refs.content.offsetHeight).toBe(100) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 970ee29c1..f4401ad8c 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -2046,7 +2046,7 @@ class TextEditorComponent { } measureCharacterDimensions () { - this.measurements.lineHeight = this.refs.characterMeasurementLine.getBoundingClientRect().height + this.measurements.lineHeight = Math.max(1, this.refs.characterMeasurementLine.getBoundingClientRect().height) this.measurements.baseCharacterWidth = this.refs.normalWidthCharacterSpan.getBoundingClientRect().width this.measurements.doubleWidthCharacterWidth = this.refs.doubleWidthCharacterSpan.getBoundingClientRect().width this.measurements.halfWidthCharacterWidth = this.refs.halfWidthCharacterSpan.getBoundingClientRect().width From 8cbfcf6e2b6f18026818517fbb445e5080fe54d5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 12 Aug 2017 15:50:35 +0200 Subject: [PATCH 32/38] Use default cursor on dummy scrollbars and make them 15px wide/tall --- src/text-editor-component.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 970ee29c1..0b1570a24 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -2851,20 +2851,22 @@ class DummyScrollbarComponent { outerStyle.bottom = 0 outerStyle.left = 0 outerStyle.right = right + 'px' - outerStyle.height = '20px' + outerStyle.height = '15px' outerStyle.overflowY = 'hidden' outerStyle.overflowX = this.props.forceScrollbarVisible ? 'scroll' : 'auto' - innerStyle.height = '20px' + outerStyle.cursor = 'default' + innerStyle.height = '15px' innerStyle.width = (this.props.scrollWidth || 0) + 'px' } else { let bottom = (this.props.horizontalScrollbarHeight || 0) outerStyle.right = 0 outerStyle.top = 0 outerStyle.bottom = bottom + 'px' - outerStyle.width = '20px' + outerStyle.width = '15px' outerStyle.overflowX = 'hidden' outerStyle.overflowY = this.props.forceScrollbarVisible ? 'scroll' : 'auto' - innerStyle.width = '20px' + outerStyle.cursor = 'default' + innerStyle.width = '15px' innerStyle.height = (this.props.scrollHeight || 0) + 'px' } From 744b96df4601c16ebf86c456c9952872a44bd2c2 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 11 Aug 2017 16:41:39 -0600 Subject: [PATCH 33/38] Suppress composition events default prevented on previous keydown This seems like a browser bug. --- src/text-editor-component.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 5764502ca..2f5817778 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1570,6 +1570,10 @@ class TextEditorComponent { } didTextInput (event) { + // Workaround for Chromium not preventing composition events when + // preventDefault is called on the keydown event that precipitated them. + if (this.lastKeydown && this.lastKeydown.defaultPrevented) return + if (!this.isInputEnabled()) return event.stopPropagation() @@ -1626,7 +1630,6 @@ class TextEditorComponent { didKeypress (event) { this.lastKeydownBeforeKeypress = this.lastKeydown - this.lastKeydown = null // This cancels the accented character behavior if we type a key normally // with the menu open. @@ -1636,7 +1639,6 @@ class TextEditorComponent { didKeyup (event) { if (this.lastKeydownBeforeKeypress && this.lastKeydownBeforeKeypress.code === event.code) { this.lastKeydownBeforeKeypress = null - this.lastKeydown = null } } @@ -1662,6 +1664,10 @@ class TextEditorComponent { } didCompositionUpdate (event) { + // Workaround for Chromium not preventing composition events when + // preventDefault is called on the keydown event that precipitated them. + if (this.lastKeydown && this.lastKeydown.defaultPrevented) return + if (this.getChromeVersion() === 56) { process.nextTick(() => { if (this.compositionCheckpoint) { From f82ff9c0d125d475909961ec44bb54cd962357c3 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 12 Aug 2017 18:09:22 -0600 Subject: [PATCH 34/38] :arrow_up: language-css --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8bcb93f7e..6949a69d1 100644 --- a/package.json +++ b/package.json @@ -139,7 +139,7 @@ "language-clojure": "0.22.4", "language-coffee-script": "0.48.9", "language-csharp": "0.14.2", - "language-css": "0.42.3", + "language-css": "0.42.4", "language-gfm": "0.90.0", "language-git": "0.19.1", "language-go": "0.44.2", From 38978da0fadff72140de6b32e016a0e306b39a43 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 13 Aug 2017 14:03:58 -0600 Subject: [PATCH 35/38] Explicitly compare compositionCheckpoint against null, since it can be 0 --- src/text-editor-component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 2f5817778..403f24bbd 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1670,7 +1670,7 @@ class TextEditorComponent { if (this.getChromeVersion() === 56) { process.nextTick(() => { - if (this.compositionCheckpoint) { + if (this.compositionCheckpoint != null) { const previewText = this.getHiddenInput().value this.props.model.insertText(previewText, {select: true}) } From 91b7c14281e88929afa6cdb36f339c20c39c9590 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Aug 2017 12:21:19 +0200 Subject: [PATCH 36/38] Prompt user only once when quitting/restarting and canceling save dialog I think this slipped through during the refactoring performed in dc32018. With this commit we are fixing the regression and adding a new main process regression test to exercise this behavior. --- spec/main-process/atom-application.test.js | 64 +++++++++++++++++----- src/main-process/atom-application.coffee | 13 ++++- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index 1e5ae0a86..80e99adc0 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -14,10 +14,11 @@ const ATOM_RESOURCE_PATH = path.resolve(__dirname, '..', '..') describe('AtomApplication', function () { this.timeout(60 * 1000) - let originalAppQuit, originalAtomHome, atomApplicationsToDestroy + let originalAppQuit, originalShowMessageBox, originalAtomHome, atomApplicationsToDestroy beforeEach(function () { originalAppQuit = electron.app.quit + originalShowMessageBox = electron.dialog mockElectronAppQuit() originalAtomHome = process.env.ATOM_HOME process.env.ATOM_HOME = makeTempDir('atom-home') @@ -39,6 +40,7 @@ describe('AtomApplication', function () { } await clearElectronSession() electron.app.quit = originalAppQuit + electron.dialog.showMessageBox = originalShowMessageBox }) describe('launch', function () { @@ -462,20 +464,42 @@ describe('AtomApplication', function () { }) }) - describe('before quitting', function () { - it('waits until all the windows have saved their state and then quits', async function () { - const dirAPath = makeTempDir("a") - const dirBPath = makeTempDir("b") - const atomApplication = buildAtomApplication() - const window1 = atomApplication.launch(parseCommandLine([path.join(dirAPath, 'file-a')])) - await focusWindow(window1) - const window2 = atomApplication.launch(parseCommandLine([path.join(dirBPath, 'file-b')])) - await focusWindow(window2) - electron.app.quit() - assert(!electron.app.hasQuitted()) - await Promise.all([window1.lastPrepareToUnloadPromise, window2.lastPrepareToUnloadPromise]) - assert(electron.app.hasQuitted()) + it('waits until all the windows have saved their state before quitting', async function () { + const dirAPath = makeTempDir("a") + const dirBPath = makeTempDir("b") + const atomApplication = buildAtomApplication() + const window1 = atomApplication.launch(parseCommandLine([path.join(dirAPath, 'file-a')])) + await focusWindow(window1) + const window2 = atomApplication.launch(parseCommandLine([path.join(dirBPath, 'file-b')])) + await focusWindow(window2) + electron.app.quit() + assert(!electron.app.hasQuitted()) + await Promise.all([window1.lastPrepareToUnloadPromise, window2.lastPrepareToUnloadPromise]) + assert(electron.app.hasQuitted()) + }) + + it.only('prevents quitting if user cancels when prompted to save an item', async () => { + const atomApplication = buildAtomApplication() + const window1 = atomApplication.launch(parseCommandLine([])) + const window2 = atomApplication.launch(parseCommandLine([])) + await Promise.all([window1.loadedPromise, window2.loadedPromise]) + await evalInWebContents(window1.browserWindow.webContents, function (sendBackToMainProcess) { + atom.workspace.getActiveTextEditor().insertText('unsaved text') + sendBackToMainProcess() }) + + // Choosing "Cancel" + mockElectronShowMessageBox({choice: 1}) + electron.app.quit() + await atomApplication.lastBeforeQuitPromise + assert(!electron.app.hasQuitted()) + assert.equal(electron.app.quit.callCount, 1) // Ensure choosing "Cancel" doesn't try to quit the electron app more than once (regression) + + // Choosing "Don't save" + mockElectronShowMessageBox({choice: 2}) + electron.app.quit() + await atomApplication.lastBeforeQuitPromise + assert(electron.app.hasQuitted()) }) function buildAtomApplication () { @@ -496,6 +520,12 @@ describe('AtomApplication', function () { function mockElectronAppQuit () { let quitted = false electron.app.quit = function () { + if (electron.app.quit.callCount) { + electron.app.quit.callCount++ + } else { + electron.app.quit.callCount = 1 + } + let shouldQuit = true electron.app.emit('before-quit', {preventDefault: () => { shouldQuit = false }}) if (shouldQuit) { @@ -507,6 +537,12 @@ describe('AtomApplication', function () { } } + function mockElectronShowMessageBox ({choice}) { + electron.dialog.showMessageBox = function () { + return choice + } + } + function makeTempDir (name) { const temp = require('temp').track() return fs.realpathSync(temp.mkdirSync(name)) diff --git a/src/main-process/atom-application.coffee b/src/main-process/atom-application.coffee index 8e8fb8b55..dcc7c6513 100644 --- a/src/main-process/atom-application.coffee +++ b/src/main-process/atom-application.coffee @@ -269,10 +269,19 @@ class AtomApplication @openPathOnEvent('application:open-license', path.join(process.resourcesPath, 'LICENSE.md')) @disposable.add ipcHelpers.on app, 'before-quit', (event) => - unless @quitting + resolveBeforeQuitPromise = null + @lastBeforeQuitPromise = new Promise((resolve) -> resolveBeforeQuitPromise = resolve) + if @quitting + resolveBeforeQuitPromise() + else event.preventDefault() @quitting = true - Promise.all(@windows.map((window) -> window.prepareToUnload())).then(-> app.quit()) + windowUnloadPromises = @windows.map((window) -> window.prepareToUnload()) + Promise.all(windowUnloadPromises).then((windowUnloadedResults) -> + didUnloadAllWindows = windowUnloadedResults.every((didUnloadWindow) -> didUnloadWindow) + app.quit() if didUnloadAllWindows + resolveBeforeQuitPromise() + ) @disposable.add ipcHelpers.on app, 'will-quit', => @killAllProcesses() From e50a73b033395b3a1b7b95f5f2c136a6abf89f73 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Aug 2017 12:28:19 +0200 Subject: [PATCH 37/38] Fix tests --- spec/main-process/atom-application.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index 80e99adc0..62fae82b3 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -18,7 +18,7 @@ describe('AtomApplication', function () { beforeEach(function () { originalAppQuit = electron.app.quit - originalShowMessageBox = electron.dialog + originalShowMessageBox = electron.dialog.showMessageBox mockElectronAppQuit() originalAtomHome = process.env.ATOM_HOME process.env.ATOM_HOME = makeTempDir('atom-home') @@ -478,7 +478,7 @@ describe('AtomApplication', function () { assert(electron.app.hasQuitted()) }) - it.only('prevents quitting if user cancels when prompted to save an item', async () => { + it('prevents quitting if user cancels when prompted to save an item', async () => { const atomApplication = buildAtomApplication() const window1 = atomApplication.launch(parseCommandLine([])) const window2 = atomApplication.launch(parseCommandLine([])) From eb1eeb3fde80d812e528df41fe34a5b0f9afdd9f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 14 Aug 2017 15:43:48 +0200 Subject: [PATCH 38/38] Ignore clicks on block decorations Previously, clicking on a block decoration to interact with it would cause the editor to scroll to the line next to it. This is inconvenient, especially if the decoration was designed to be interactive and contained buttons or links. If the decoration was close to the bottom of the screen, clicking on a button inside of it would make the editor scroll down and abort the click. This behavior regressed during the editor rendering layer rewrite and with this commit we are restoring the original behavior by simply ignoring clicks that land on block decorations. --- spec/text-editor-component-spec.js | 33 ++++++++++++++++++++++++++++++ src/text-editor-component.js | 12 +++++++++++ 2 files changed, 45 insertions(+) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index a4631b541..4c0108b33 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -2153,6 +2153,39 @@ describe('TextEditorComponent', () => { expect(component.refs.blockDecorationMeasurementArea.offsetWidth).toBe(component.getScrollWidth()) }) + it('does not change the cursor position when clicking on a block decoration', async () => { + const {editor, component} = buildComponent() + + const decorationElement = document.createElement('div') + decorationElement.textContent = 'Parent' + const childElement = document.createElement('div') + childElement.textContent = 'Child' + decorationElement.appendChild(childElement) + const marker = editor.markScreenPosition([4, 0]) + editor.decorateMarker(marker, {type: 'block', item: decorationElement}) + await component.getNextUpdatePromise() + + const decorationElementClientRect = decorationElement.getBoundingClientRect() + component.didMouseDownOnContent({ + target: decorationElement, + detail: 1, + button: 0, + clientX: decorationElementClientRect.left, + clientY: decorationElementClientRect.top + }) + expect(editor.getCursorScreenPosition()).toEqual([0, 0]) + + const childElementClientRect = childElement.getBoundingClientRect() + component.didMouseDownOnContent({ + target: childElement, + detail: 1, + button: 0, + clientX: childElementClientRect.left, + clientY: childElementClientRect.top + }) + expect(editor.getCursorScreenPosition()).toEqual([0, 0]) + }) + function createBlockDecorationAtScreenRow(editor, screenRow, {height, margin, position}) { const marker = editor.markScreenPosition([screenRow, 0], {invalidate: 'never'}) const item = document.createElement('div') diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 403f24bbd..e409001a8 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1689,6 +1689,18 @@ class TextEditorComponent { const {target, button, detail, ctrlKey, shiftKey, metaKey} = event const platform = this.getPlatform() + // Ignore clicks on block decorations. + if (target) { + let element = target + while (element && element !== this.element) { + if (this.blockDecorationsByElement.has(element)) { + return + } + + element = element.parentElement + } + } + // On Linux, position the cursor on middle mouse button click. A // textInput event with the contents of the selection clipboard will be // dispatched by the browser automatically on mouseup.