Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Update CodeMirror for sprint 32. Avoid hang by setting width on mock editor holder in unit tests. #5283

Merged
merged 2 commits into from
Sep 23, 2013

Conversation

njx
Copy link

@njx njx commented Sep 21, 2013

@RaymondLim

See comments in #5179. I verified that all the automated tests pass and smoke tests are ok after this change.

Could you also verify that the issues caused by codemirror/codemirror5#1787 are also fixed after this update, and close that bug as well as our own related issues? Thanks.

@ghost ghost assigned RaymondLim Sep 21, 2013
@@ -406,7 +406,7 @@ define(function (require, exports, module) {
function createMockEditorForDocument(doc, visibleRange) {
// Initialize EditorManager/PanelManager and position the editor-holder offscreen
// (".content" may not exist, but that's ok for headless tests where editor height doesn't matter)
var $editorHolder = createMockElement().attr("id", "mock-editor-holder");
var $editorHolder = createMockElement().css("width", "1000px").attr("id", "mock-editor-holder");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change indeed fixes the hang issue.

@njx
Copy link
Author

njx commented Sep 23, 2013

I updated to the latest upstream, re-ran the unit tests and did a little smoke testing. When running all the unit tests I ran into one failure in MultiRangeInlineEditor-test, but it didn't repro when clicking on it by itself. Looking at the test, it seems like there's potentially some async issues with the tests themselves (they don't wait for async opening/closes to finish, and the animations for the inline editors actually don't exist in the test runner window, similar to other bugs we've had). So I don't think that's related to the latest update. I'll file a separate bug on these

I"m going to go ahead and merge this since it seems okay.

njx pushed a commit that referenced this pull request Sep 23, 2013
Update CodeMirror for sprint 32. Avoid hang by setting width on mock editor holder in unit tests.
@njx njx merged commit 40bbcb2 into master Sep 23, 2013
@njx njx deleted the nj/update-cm-sprint-32 branch September 23, 2013 20:20
@njx
Copy link
Author

njx commented Sep 23, 2013

@RaymondLim - can you double-check again that the original bug is still fixed? (I just realized I didn't retest it after pulling the new CM.) If so, please close it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants