From 5dc64893594ada7be58f53e2a2d13ab9753cb31c Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Fri, 5 Apr 2013 14:57:27 -0700 Subject: [PATCH 1/5] Cleanups, mainly to unit tests: - Improve docs in SpecRunnerUtils - Add assertion in SRU in case a mock Document is created without an Editor but then used in a way that would auto-create an Editor (which won't get cleaned up) - Add assertion in _destroyEditorIfUnneeded() since its name makes it easy to call with the wrong type (including via SRU.destroyMockEditor()) - Rename HTML code hints suite & remove unused var - Minor docs cleanups in LanguageManager (incorrect usage of "i.e.", missing docs for new fileNames option, etc.) - Add unit tests for install dialog cases where installer comes back with a result after clicking Cancel but before Cancel times out, or comes back vary late after timeout once user has opted to close the dialog - Remove a duplicated LanguageManager test (due to f67cfcdf) - Add a few more StringMatch tests to ensure tricky cases don't break in the opposite direction as before --- src/editor/EditorManager.js | 3 + .../default/HTMLCodeHints/unittests.js | 5 +- src/language/LanguageManager.js | 23 ++--- test/spec/InstallExtensionDialog-test.js | 97 ++++++++++++++++++- test/spec/LanguageManager-test.js | 36 +------ test/spec/SpecRunnerUtils.js | 33 +++++-- test/spec/StringMatch-test.js | 6 ++ 7 files changed, 144 insertions(+), 59 deletions(-) diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index 577ec9ccab7..d64910a4c5f 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -295,6 +295,9 @@ define(function (require, exports, module) { var editor = document._masterEditor; if (!editor) { + if (!(document instanceof DocumentManager.Document)) { + throw new Error("_destroyEditorIfUnneeded() takes a Document arg, not an Editor"); + } return; } diff --git a/src/extensions/default/HTMLCodeHints/unittests.js b/src/extensions/default/HTMLCodeHints/unittests.js index f5311d943f4..4ab18eba61b 100644 --- a/src/extensions/default/HTMLCodeHints/unittests.js +++ b/src/extensions/default/HTMLCodeHints/unittests.js @@ -34,7 +34,7 @@ define(function (require, exports, module) { CodeHintManager = brackets.getModule("editor/CodeHintManager"), HTMLCodeHints = require("main"); - describe("HTML Attribute Hinting", function () { + describe("HTML Code Hinting", function () { var defaultContent = "\n" + "\n" + @@ -49,7 +49,6 @@ define(function (require, exports, module) { "\n" + "\n"; - var testWindow; var testDocument, testEditor; beforeEach(function () { @@ -682,5 +681,5 @@ define(function (require, exports, module) { }); - }); // describe("HTML Attribute Hinting" + }); // describe("HTML Code Hinting" }); diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index a5db2238c43..a9161978860 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -173,7 +173,7 @@ define(function (require, exports, module) { /** * Resolves a language ID to a Language object. * File names have a higher priority than file extensions. - * @param {!string} id Identifier for this language, use only letters a-z or digits 0-9 and _ inbetween (i.e. "cpp", "foo_bar", "c99") + * @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") * @return {Language} The language with the provided identifier or undefined */ function getLanguage(id) { @@ -320,7 +320,7 @@ define(function (require, exports, module) { /** * Sets the identifier for this language or prints an error to the console. - * @param {!string} id Identifier for this language, use only letters a-z or digits 0-9, and _ inbetween (i.e. "cpp", "foo_bar", "c99") + * @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") * @return {boolean} Whether the ID was valid and set or not */ Language.prototype._setId = function (id) { @@ -348,7 +348,7 @@ define(function (require, exports, module) { /** * Sets the human-readable name of this language or prints an error to the console. - * @param {!string} name Human-readable name of the language, as it's commonly referred to (i.e. "C++") + * @param {!string} name Human-readable name of the language, as it's commonly referred to (e.g. "C++") * @return {boolean} Whether the name was valid and set or not */ Language.prototype._setName = function (name) { @@ -371,7 +371,7 @@ define(function (require, exports, module) { /** * Loads a mode and sets it for this language. * - * @param {string|Array.} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] + * @param {string|Array.} mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. * * @return {$.Promise} A promise object that will be resolved when the mode is loaded and set @@ -523,7 +523,7 @@ define(function (require, exports, module) { /** * Sets the prefixes to use for line comments in this language or prints an error to the console. * @param {!string|Array.} prefix Prefix string or and array of prefix strings - * to use for line comments (i.e. "//" or ["//", "#"]) + * to use for line comments (e.g. "//" or ["//", "#"]) * @return {boolean} Whether the syntax was valid and set or not */ Language.prototype.setLineCommentSyntax = function (prefix) { @@ -641,13 +641,14 @@ define(function (require, exports, module) { /** * Defines a language. * - * @param {!string} id Unique identifier for this language, use only letters a-z or digits 0-9, and _ inbetween (i.e. "cpp", "foo_bar", "c99") + * @param {!string} id Unique identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") * @param {!Object} definition An object describing the language - * @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (i.e. "C++") - * @param {Array.} definition.fileExtensions List of file extensions used by this language (i.e. ["php", "php3"]) - * @param {Array.} definition.blockComment Array with two entries defining the block comment prefix and suffix (i.e. [""]) - * @param {string|Array.} definition.lineComment Line comment prefixes (i.e. "//" or ["//", "#"]) - * @param {string|Array.} definition.mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] + * @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (e.g. "C++") + * @param {Array.} definition.fileExtensions List of file extensions used by this language (e.g. ["php", "php3"]). May contain periods (e.g. ["coffee.md"]) + * @param {Array.} definition.fileNames List of file names (e.g. ["Makefile"] or ["package.json]) + * @param {Array.} definition.blockComment Array with two entries defining the block comment prefix and suffix (e.g. [""]) + * @param {string|Array.} definition.lineComment Line comment prefixes (e.g. "//" or ["//", "#"]) + * @param {string|Array.} definition.mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. * * @return {$.Promise} A promise object that will be resolved with a Language object diff --git a/test/spec/InstallExtensionDialog-test.js b/test/spec/InstallExtensionDialog-test.js index aefdbf1f7de..d5989301a5b 100644 --- a/test/spec/InstallExtensionDialog-test.js +++ b/test/spec/InstallExtensionDialog-test.js @@ -495,6 +495,7 @@ define(function (require, exports, module) { }); }); + it("should close when hitting Esc after timing out if cancel doesn't complete quickly", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -512,7 +513,20 @@ define(function (require, exports, module) { }); }); - it("should keep close button enabled and not throw an exception if install succeeds after cancelation", function () { + it("should keep close button enabled and not throw an exception if install succeeds quickly after cancelation", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + deferred.resolve(); + expect(fields.$okButton.attr("disabled")).toBeFalsy(); + }); + }); + + it("should keep close button enabled and not throw an exception if install succeeds slowly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); runs(function () { @@ -527,8 +541,36 @@ define(function (require, exports, module) { expect(fields.$okButton.attr("disabled")).toBeFalsy(); }); }); - - it("should keep close button enabled and not throw an exception if install fails after cancelation", function () { + it("should keep close button enabled and not throw an exception if install succeeds after cancelation & force close", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + }); + waits(100); + runs(function () { + fields.$okButton.click(); + deferred.resolve(); + expect(fields.$dlg.is(":visible")).toBeFalsy(); + }); + }); + + it("should keep close button enabled and not throw an exception if install fails quickly after cancelation", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + deferred.reject(); + expect(fields.$okButton.attr("disabled")).toBeFalsy(); + }); + }); + it("should keep close button enabled and not throw an exception if install fails slowly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); runs(function () { @@ -544,7 +586,37 @@ define(function (require, exports, module) { }); }); - it("should keep close button enabled and not throw an exception if install cancelation completes after cancelation", function () { + it("should keep close button enabled and not throw an exception if install fails after cancelation & force close", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + }); + waits(100); + runs(function () { + fields.$okButton.click(); + deferred.reject(); + expect(fields.$dlg.is(":visible")).toBeFalsy(); + }); + }); + + it("should keep close button enabled and not throw an exception if install cancelation completes quickly after cancelation", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + deferred.reject("CANCELED"); + expect(fields.$okButton.attr("disabled")).toBeFalsy(); + }); + }); + + it("should keep close button enabled and not throw an exception if install cancelation completes slowly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); runs(function () { @@ -558,6 +630,23 @@ define(function (require, exports, module) { deferred.reject("CANCELED"); expect(fields.$okButton.attr("disabled")).toBeFalsy(); }); + }); + + it("should keep close button enabled and not throw an exception if install cancelation completes after cancelation & force close", function () { + var deferred = new $.Deferred(), + installer = makeInstaller(null, deferred); + runs(function () { + setUrl(); + dialog._cancelTimeout = 50; + fields.$okButton.click(); + fields.$cancelButton.click(); + }); + waits(100); + runs(function () { + fields.$okButton.click(); + deferred.reject("CANCELED"); + expect(fields.$dlg.is(":visible")).toBeFalsy(); + }); // This must be in the last spec in the suite. runs(function () { diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index f2171c64d61..421b24faa00 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -386,7 +386,7 @@ define(function (require, exports, module) { it("should update the document's language when a file is renamed", function () { var javascript = LanguageManager.getLanguage("javascript"), html = LanguageManager.getLanguage("html"), - doc = SpecRunnerUtils.createMockDocument("foo", javascript), + doc = SpecRunnerUtils.createMockActiveDocument({ filename: "foo.js", language: "javascript" }), spy = jasmine.createSpy("languageChanged event handler"); // sanity check language @@ -394,40 +394,6 @@ define(function (require, exports, module) { // Documents are only 'active' while referenced; they won't be maintained by DocumentManager // for global updates like rename otherwise. - // Undo createMockDocument()'s shimming to allow this. - doc.addRef = DocumentManager.Document.prototype.addRef; - doc.releaseRef = DocumentManager.Document.prototype.releaseRef; - doc.addRef(); - - // listen for event - $(doc).on("languageChanged", spy); - - // trigger a rename - DocumentManager.notifyPathNameChanged(doc.file.name, "dummy.html", false); - - // language should change - expect(doc.getLanguage()).toBe(html); - expect(spy).toHaveBeenCalled(); - expect(spy.callCount).toEqual(1); - - // check callback args (arg 0 is a jQuery event) - expect(spy.mostRecentCall.args[1]).toBe(javascript); - expect(spy.mostRecentCall.args[2]).toBe(html); - - // cleanup - doc.releaseRef(); - }); - - it("should update the document's language when a language is added", function () { - var javascript = LanguageManager.getLanguage("javascript"), - html = LanguageManager.getLanguage("html"), - doc = SpecRunnerUtils.createMockActiveDocument({ filename: "foo.js", language: "javascript" }), - spy = jasmine.createSpy("languageChanged event handler"); - - // sanity check language - expect(doc.getLanguage()).toBe(javascript); - - // make active doc.addRef(); // listen for event diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index 518ff8849c4..f20f2610206 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -142,8 +142,14 @@ define(function (require, exports, module) { }; /** - * Returns a Document suitable for use with an Editor in isolation, but - * maintained active for global updates like name and language changes. + * Returns a Document suitable for use with an Editor in isolation, but that can be registered with + * DocumentManager via addRef() so it is maintained for global updates like name and language changes. + * + * Like a normal Document, if you cause an addRef() on this you MUST call releaseRef() later. + * + * @param {!{language:?string, filename:?string, content:?string }} options + * Language defaults to JavaScript, filename defaults to a placeholder name, and + * content defaults to "". */ function createMockActiveDocument(options) { var language = options.language || LanguageManager.getLanguage("javascript"), @@ -173,6 +179,11 @@ define(function (require, exports, module) { /** * Returns a Document suitable for use with an Editor in isolation: i.e., a Document that will * never be set as the currentDocument or added to the working set. + * + * Unlike a real Document, does NOT need to be explicitly cleaned up. + * + * @param {?string} initialContent Defaults to "" + * @param {?string} languageId Defaults to JavaScript */ function createMockDocument(initialContent, languageId) { var language = LanguageManager.getLanguage(languageId) || LanguageManager.getLanguage("javascript"), @@ -183,14 +194,23 @@ define(function (require, exports, module) { // fails to clean up properly (if test fails, or due to an apparent bug with afterEach()) docToShim.addRef = function () {}; docToShim.releaseRef = function () {}; + docToShim._ensureMasterEditor = function () { + if (!this._masterEditor) { + // Don't let Document create an Editor itself via EditorManager; the unit test can't clean that up + throw new Error("Use create/destroyMockEditor() to test edit operations"); + } + }; return docToShim; } /** - * Returns a Document and Editor suitable for use with an Editor in - * isolation: i.e., a Document that will never be set as the - * currentDocument or added to the working set. + * Returns a Document and Editor suitable for use with an Editor in isolation: i.e., a + * Document that will never be set as the currentDocument or added to the working set. + * The Editor *will* be reported as the "active editor" by EditorManager, however. + * + * Must be cleaned up by calling destroyMockEditor(document) later. + * * @return {!{doc:{Document}, editor:{Editor}}} */ function createMockEditor(initialContent, languageId, visibleRange) { @@ -216,9 +236,10 @@ define(function (require, exports, module) { /** * Destroy the Editor instance for a given mock Document. - * @param {!Document} doc + * @param {!Document} doc Document whose master editor to destroy */ function destroyMockEditor(doc) { + EditorManager._notifyActiveEditorChanged(null); EditorManager._destroyEditorIfUnneeded(doc); // Clear editor holder so EditorManager doesn't try to resize destroyed object diff --git a/test/spec/StringMatch-test.js b/test/spec/StringMatch-test.js index ff640be1653..6eca2d65475 100644 --- a/test/spec/StringMatch-test.js +++ b/test/spec/StringMatch-test.js @@ -639,14 +639,20 @@ define(function (require, exports, module) { // Object.prototype has toString var toStringResult = matcher.match("toString", "t"); expect(toStringResult).toBeTruthy(); + toStringResult = matcher.match("toString", "x"); + expect(toStringResult).toBeFalsy(); // Array.prototype has length var lengthResult = matcher.match("length", "l"); expect(lengthResult).toBeTruthy(); + lengthResult = matcher.match("length", "x"); + expect(lengthResult).toBeFalsy(); // Object.prototype has hasOwnProperty var hasOwnPropertyResult = matcher.match("hasOwnProperty", "h"); expect(hasOwnPropertyResult).toBeTruthy(); + hasOwnPropertyResult = matcher.match("hasOwnProperty", "x"); + expect(hasOwnPropertyResult).toBeFalsy(); }); it("can reset the caches", function () { From 045fae6fd2a841b120c481308a159f3950ab2faf Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Fri, 12 Apr 2013 14:34:00 -0700 Subject: [PATCH 2/5] Cleanups: * DocumentManager: use CollectionUtils.forEach() in notifyPathNameChanged() (removing early-exit optimization that's probably unneeded; can reintroduce when CollectionUtils.some() lands) * Move filename->language mapping tests from Editor-test to LanguageManager-test * Add another #330 testcase as suggested by Glenn in #3064 * Add StringMatch testcases as suggested by Kevin in #3417 * SpecRunnerUtils: use waitsForDone() more * CSSUtils-test: remove unused vars * Dialogs: tiny docs improvement --- src/document/DocumentManager.js | 37 +++++++++++----------------- src/widgets/Dialogs.js | 2 +- test/spec/CSSUtils-test.js | 6 +---- test/spec/Editor-test.js | 34 +++++--------------------- test/spec/FileIndexManager-test.js | 39 ++++++++++++++++++++++++++++-- test/spec/LanguageManager-test.js | 10 ++++++++ test/spec/SpecRunnerUtils.js | 13 +++------- test/spec/StringMatch-test.js | 6 +++++ 8 files changed, 79 insertions(+), 68 deletions(-) diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index c981d6ad1b2..37dea3532c5 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -1161,34 +1161,25 @@ define(function (require, exports, module) { * @param {boolean} isFolder True if path is a folder; False if it is a file. */ function notifyPathNameChanged(oldName, newName, isFolder) { - var i, path; + var i; // Update open documents. This will update _currentDocument too, since // the current document is always open. var keysToDelete = []; - for (path in _openDocuments) { - if (_openDocuments.hasOwnProperty(path)) { - if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) { - var doc = _openDocuments[path]; - - // Copy value to new key - var newKey = path.replace(oldName, newName); - _openDocuments[newKey] = doc; - - keysToDelete.push(path); - - // Update document file - FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder); - doc._notifyFilePathChanged(); - - if (!isFolder) { - // If the path name is a file, there can only be one matched entry in the open document - // list, which we just updated. Break out of the for .. in loop. - break; - } - } + CollectionUtils.forEach(_openDocuments, function (doc, path) { + if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) { + // Copy value to new key + var newKey = path.replace(oldName, newName); + _openDocuments[newKey] = doc; + + keysToDelete.push(path); + + // Update document file + FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder); + doc._notifyFilePathChanged(); } - } + }); + // Delete the old keys for (i = 0; i < keysToDelete.length; i++) { delete _openDocuments[keysToDelete[i]]; diff --git a/src/widgets/Dialogs.js b/src/widgets/Dialogs.js index c95f4afa69a..4bab285a22e 100644 --- a/src/widgets/Dialogs.js +++ b/src/widgets/Dialogs.js @@ -215,7 +215,7 @@ define(function (require, exports, module) { * -- the HTML for the dialog contains elements with "title" and "message" classes, as well as a number * of elements with "dialog-button" class, each of which has a "data-button-id". * - * @param {string} dlgClass The class of the dialog node in the HTML. + * @param {string} dlgClass The class of the dialog node in the HTML. Typically one of DIALOG_ID_*. * @param {string=} title The title of the error dialog. Can contain HTML markup. If unspecified, title in * the HTML template is used unchanged. * @param {string=} message The message to display in the error dialog. Can contain HTML markup. If diff --git a/test/spec/CSSUtils-test.js b/test/spec/CSSUtils-test.js index 1b31dc230e1..a7f00afd84b 100644 --- a/test/spec/CSSUtils-test.js +++ b/test/spec/CSSUtils-test.js @@ -1404,18 +1404,14 @@ define(function (require, exports, module) { var testPath = SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"), CSSUtils, DocumentManager, - FileViewController, - ProjectManager, - brackets; + FileViewController; beforeEach(function () { SpecRunnerUtils.createTestWindowAndRun(this, function (testWindow) { // Load module instances from brackets.test - brackets = testWindow.brackets; CSSUtils = testWindow.brackets.test.CSSUtils; DocumentManager = testWindow.brackets.test.DocumentManager; FileViewController = testWindow.brackets.test.FileViewController; - ProjectManager = testWindow.brackets.test.ProjectManager; SpecRunnerUtils.loadProjectInTestWindow(testPath); }); diff --git a/test/spec/Editor-test.js b/test/spec/Editor-test.js index ed6a9f2589f..761c81a7fd6 100644 --- a/test/spec/Editor-test.js +++ b/test/spec/Editor-test.js @@ -56,7 +56,7 @@ define(function (require, exports, module) { } describe("Editor", function () { - var defaultContent = 'Brackets is going to be awesome!\n'; + var defaultContent = "Brackets is going to be awesome!\n"; var myDocument, myEditor; function createTestEditor(content, languageId) { @@ -110,36 +110,14 @@ define(function (require, exports, module) { myEditor._codeMirror.setValue("new content"); expect(changeFired).toBe(true); }); - - }); - - describe("File extension to mode mapping", function () { - - it("should switch to the HTML mode for files ending in .html", function () { - // verify editor content - var mode = LanguageManager.getLanguageForPath("file:///only/testing/the/path.html").getMode(); - expect(mode).toSpecifyModeNamed("text/x-brackets-html"); - }); - - it("should switch modes even if the url has a query string", function () { - // verify editor content - var mode = LanguageManager.getLanguageForPath("http://only.org/testing/the/path.css?v=2").getMode(); - expect(mode).toSpecifyModeNamed(langNames.css.mode); - }); - it("should accept just a file name too", function () { - // verify editor content - var mode = LanguageManager.getLanguageForPath("path.js").getMode(); - expect(mode).toSpecifyModeNamed(langNames.javascript.mode); - }); - - it("should default to plain text for unknown file extensions", function () { - // verify editor content - var mode = LanguageManager.getLanguageForPath("test.foo").getMode(); + it("should set mode based on Document language", function () { + createTestEditor(defaultContent, "html"); - // "unknown" mode uses it's MIME type instead - expect(mode).toBe("text/plain"); + var htmlLanguage = LanguageManager.getLanguage("html"); + expect(myEditor.getModeForDocument()).toBe(htmlLanguage.getMode()); }); + }); describe("Focus", function () { diff --git a/test/spec/FileIndexManager-test.js b/test/spec/FileIndexManager-test.js index 7159ea3689e..9eeea3501df 100644 --- a/test/spec/FileIndexManager-test.js +++ b/test/spec/FileIndexManager-test.js @@ -89,7 +89,7 @@ define(function (require, exports, module) { }); - it("should handle simultaneous requests without doing extra work", function () { // #330 + it("should handle identical simultaneous requests without doing extra work", function () { // #330 // Open a directory SpecRunnerUtils.loadProjectInTestWindow(testPath); @@ -105,7 +105,7 @@ define(function (require, exports, module) { .done(function (result) { allFiles1 = result; }); - promise2 = FileIndexManager.getFileInfoList("all") + promise2 = FileIndexManager.getFileInfoList("all") // same request again .done(function (result) { allFiles2 = result; }); @@ -124,6 +124,41 @@ define(function (require, exports, module) { }); }); + it("should handle differing simultaneous requests without doing extra work", function () { // #330 + // Open a directory + SpecRunnerUtils.loadProjectInTestWindow(testPath); + + var projectRoot; + var promise1, promise2; + var allFiles1, allFiles2; + runs(function () { + projectRoot = ProjectManager.getProjectRoot(); + spyOn(projectRoot, "createReader").andCallThrough(); + + // Kick off two index requests in parallel + promise1 = FileIndexManager.getFileInfoList("all") + .done(function (result) { + allFiles1 = result; + }); + promise2 = FileIndexManager.getFileInfoList("css") // different request in parallel + .done(function (result) { + allFiles2 = result; + }); + + waitsForDone(promise1, "First FileIndexManager.getFileInfoList()"); + waitsForDone(promise2, "Second FileIndexManager.getFileInfoList()"); + }); + + runs(function () { + // Correct response to both promises + expect(allFiles1.length).toEqual(8); + expect(allFiles2.length).toEqual(3); + + // Didn't scan project tree twice + expect(projectRoot.createReader.callCount).toBe(1); + }); + }); + it("should match a specific filename and return the correct FileInfo", function () { // Open a directory SpecRunnerUtils.loadProjectInTestWindow(testPath); diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 421b24faa00..7bf2fdac322 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -129,11 +129,21 @@ define(function (require, exports, module) { it("should map file extensions to languages", function () { var html = LanguageManager.getLanguage("html"), + css = LanguageManager.getLanguage("css"), unknown = LanguageManager.getLanguage("unknown"); + // Bare file names expect(LanguageManager.getLanguageForPath("foo.html")).toBe(html); expect(LanguageManager.getLanguageForPath("INDEX.HTML")).toBe(html); expect(LanguageManager.getLanguageForPath("foo.doesNotExist")).toBe(unknown); + + // URIs + expect(LanguageManager.getLanguageForPath("file:///only/testing/the/path.html")).toBe(html); + expect(LanguageManager.getLanguageForPath("http://only.org/testing/the/path.css?v=2")).toBe(css); + + // Things that aren't extensions + expect(LanguageManager.getLanguageForPath("/code/html")).toBe(unknown); + expect(LanguageManager.getLanguageForPath("/code/foo.html.notreally")).toBe(unknown); }); it("should map complex file extensions to languages", function () { diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index f20f2610206..de18d6434ec 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -338,23 +338,18 @@ define(function (require, exports, module) { dismissButton.click(); // Dialog should resolve/reject the promise - waitsForDone(promise); + waitsForDone(promise, "dismiss dialog"); } function loadProjectInTestWindow(path) { - var isReady = false; - runs(function () { // begin loading project path var result = _testWindow.brackets.test.ProjectManager.openProject(path); - result.done(function () { - isReady = true; - }); + + // wait for file system to finish loading + waitsForDone(result, "ProjectManager.openProject()"); }); - - // wait for file system to finish loading - waitsFor(function () { return isReady; }, "openProject() timeout", 1000); } /** diff --git a/test/spec/StringMatch-test.js b/test/spec/StringMatch-test.js index 6eca2d65475..8917cb8d947 100644 --- a/test/spec/StringMatch-test.js +++ b/test/spec/StringMatch-test.js @@ -641,18 +641,24 @@ define(function (require, exports, module) { expect(toStringResult).toBeTruthy(); toStringResult = matcher.match("toString", "x"); expect(toStringResult).toBeFalsy(); + toStringResult = matcher.match("toString", "xx"); // 2nd no-match to test _noMatchCache + expect(toStringResult).toBeFalsy(); // Array.prototype has length var lengthResult = matcher.match("length", "l"); expect(lengthResult).toBeTruthy(); lengthResult = matcher.match("length", "x"); expect(lengthResult).toBeFalsy(); + lengthResult = matcher.match("length", "xx"); // 2nd no-match to test _noMatchCache + expect(lengthResult).toBeFalsy(); // Object.prototype has hasOwnProperty var hasOwnPropertyResult = matcher.match("hasOwnProperty", "h"); expect(hasOwnPropertyResult).toBeTruthy(); hasOwnPropertyResult = matcher.match("hasOwnProperty", "x"); expect(hasOwnPropertyResult).toBeFalsy(); + hasOwnPropertyResult = matcher.match("hasOwnProperty", "xx"); // 2nd no-match to test _noMatchCache + expect(hasOwnPropertyResult).toBeFalsy(); }); it("can reset the caches", function () { From 7250b966c6fc00b1ec999bb319d792541a5b9b11 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Mon, 22 Jul 2013 21:50:58 -0700 Subject: [PATCH 3/5] Fix JSLint/JSHint errors (some of which were on master). Small docs & formatting tweaks. Fix merge cruft in Editor-test & CSSUtils-test. --- src/editor/EditorManager.js | 4 ++-- src/language/LanguageManager.js | 10 +++++----- test/spec/CSSUtils-test.js | 18 ++++++++---------- test/spec/Editor-test.js | 1 - test/spec/InstallExtensionDialog-test.js | 7 +++---- test/spec/LanguageManager-test.js | 2 +- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/editor/EditorManager.js b/src/editor/EditorManager.js index f8b238a0b10..d240ae2420b 100644 --- a/src/editor/EditorManager.js +++ b/src/editor/EditorManager.js @@ -332,7 +332,7 @@ define(function (require, exports, module) { if (!editor) { if (!(document instanceof DocumentManager.Document)) { - throw new Error("_destroyEditorIfUnneeded() takes a Document arg, not an Editor"); + throw new Error("_destroyEditorIfUnneeded() should be passed a Document"); } return; } @@ -676,7 +676,7 @@ define(function (require, exports, module) { // an inline widget's editor has focus, so close it PerfUtils.markStart(PerfUtils.INLINE_WIDGET_CLOSE); inlineWidget.close().done(function () { - PerfUtils.addMeasurement(PerfUtils.INLINE_WIDGET_CLOSE); + PerfUtils.addMeasurement(PerfUtils.INLINE_WIDGET_CLOSE); // return a resolved promise to CommandManager result.resolve(false); }); diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 0bc63e5727b..5c7bb610b36 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -175,7 +175,7 @@ define(function (require, exports, module) { /** * Resolves a language ID to a Language object. * File names have a higher priority than file extensions. - * @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") + * @param {!string} id Identifier for this language: lowercase letters, digits, and _ separators (e.g. "cpp", "foo_bar", "c99") * @return {Language} The language with the provided identifier or undefined */ function getLanguage(id) { @@ -322,7 +322,7 @@ define(function (require, exports, module) { /** * Sets the identifier for this language or prints an error to the console. - * @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") + * @param {!string} id Identifier for this language: lowercase letters, digits, and _ separators (e.g. "cpp", "foo_bar", "c99") * @return {boolean} Whether the ID was valid and set or not */ Language.prototype._setId = function (id) { @@ -637,11 +637,11 @@ define(function (require, exports, module) { /** * Defines a language. * - * @param {!string} id Unique identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99") + * @param {!string} id Unique identifier for this language: lowercase letters, digits, and _ separators (e.g. "cpp", "foo_bar", "c99") * @param {!Object} definition An object describing the language * @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (e.g. "C++") - * @param {Array.} definition.fileExtensions List of file extensions used by this language (e.g. ["php", "php3"]). May contain periods (e.g. ["coffee.md"]) - * @param {Array.} definition.fileNames List of file names (e.g. ["Makefile"] or ["package.json]) + * @param {Array.} definition.fileExtensions List of file extensions used by this language (e.g. ["php", "php3"] or ["coffee.md"] - may contain dots) + * @param {Array.} definition.fileNames List of exact file names (e.g. ["Makefile"] or ["package.json]). Higher precedence than file extension. * @param {Array.} definition.blockComment Array with two entries defining the block comment prefix and suffix (e.g. [""]) * @param {string|Array.} definition.lineComment Line comment prefixes (e.g. "//" or ["//", "#"]) * @param {string|Array.} definition.mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] diff --git a/test/spec/CSSUtils-test.js b/test/spec/CSSUtils-test.js index cd9fc0f2a80..e12d513d4c9 100644 --- a/test/spec/CSSUtils-test.js +++ b/test/spec/CSSUtils-test.js @@ -973,10 +973,10 @@ define(function (require, exports, module) { // Braces inside string; string inside rule (not inside selector) css = "a::after { content: ' {' attr(href) '}'; } \n" + - ".foo { color:red } \n" + - "a::after { content: \" {\" attr(href) \"}\"; } \n" + - "li::before { content: \"} h4 { color:black }\"; } \n" + - "div { color:green }"; + ".foo { color:red } \n" + + "a::after { content: \" {\" attr(href) \"}\"; } \n" + + "li::before { content: \"} h4 { color:black }\"; } \n" + + "div { color:green }"; result = match(css, { tag: "a" }); expect(result.length).toBe(2); @@ -999,7 +999,7 @@ define(function (require, exports, module) { expect(result.length).toBe(1); css = "@import \"null?\\\"{\"; \n" + // a real-world CSS hack similar to the above case - "div { color: red }"; + "div { color: red }"; result = match(css, { tag: "div" }); expect(result.length).toBe(1); @@ -1051,9 +1051,9 @@ define(function (require, exports, module) { expect(result.length).toBe(1); css = ".foo\n" + - "{\n" + - " color: red;\n" + - "}"; + "{\n" + + " color: red;\n" + + "}"; result = match(css, { clazz: "foo" }); expect(result.length).toBe(1); }); @@ -1425,11 +1425,9 @@ define(function (require, exports, module) { }); afterEach(function () { - brackets = null; CSSUtils = null; DocumentManager = null; FileViewController = null; - ProjectManager = null; SpecRunnerUtils.closeTestWindow(); }); diff --git a/test/spec/Editor-test.js b/test/spec/Editor-test.js index 6694910d0b3..761c81a7fd6 100644 --- a/test/spec/Editor-test.js +++ b/test/spec/Editor-test.js @@ -113,7 +113,6 @@ define(function (require, exports, module) { it("should set mode based on Document language", function () { createTestEditor(defaultContent, "html"); - var mode = LanguageManager.getLanguageForPath("only/testing/the/path.css").getMode(); var htmlLanguage = LanguageManager.getLanguage("html"); expect(myEditor.getModeForDocument()).toBe(htmlLanguage.getMode()); diff --git a/test/spec/InstallExtensionDialog-test.js b/test/spec/InstallExtensionDialog-test.js index d4e30b7696e..6bd4053df28 100644 --- a/test/spec/InstallExtensionDialog-test.js +++ b/test/spec/InstallExtensionDialog-test.js @@ -522,6 +522,7 @@ define(function (require, exports, module) { }); }); + // Cancelation vs. successful install race conditions it("should keep close button enabled and not throw an exception if install succeeds quickly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -534,7 +535,6 @@ define(function (require, exports, module) { expect(fields.$okButton.prop("disabled")).toBe(false); }); }); - it("should keep close button enabled and not throw an exception if install succeeds slowly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -567,6 +567,7 @@ define(function (require, exports, module) { }); }); + // Cancelation vs. failed install race conditions it("should keep close button enabled and not throw an exception if install fails quickly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -594,7 +595,6 @@ define(function (require, exports, module) { expect(fields.$okButton.prop("disabled")).toBe(false); }); }); - it("should stay closed and not throw an exception if install fails after cancelation & force close", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -612,6 +612,7 @@ define(function (require, exports, module) { }); }); + // Cancelation actually suceeding it("should keep close button enabled and not throw an exception if install cancelation completes quickly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -624,7 +625,6 @@ define(function (require, exports, module) { expect(fields.$okButton.prop("disabled")).toBe(false); }); }); - it("should keep close button enabled and not throw an exception if install cancelation completes slowly after cancelation", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); @@ -640,7 +640,6 @@ define(function (require, exports, module) { expect(fields.$okButton.prop("disabled")).toBe(false); }); }); - it("should stay closed and not throw an exception if install cancelation completes after cancelation & force close", function () { var deferred = new $.Deferred(), installer = makeInstaller(null, deferred); diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index b51762c4f63..b7db864dfed 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -151,7 +151,7 @@ define(function (require, exports, module) { expect(LanguageManager.getLanguageForPath("/only/testing/the/path.css")).toBe(css); // abs Mac/Linux-style expect(LanguageManager.getLanguageForPath("only/testing/the/path.css")).toBe(css); // relative - // Things that aren't extensions + // Unknown file types expect(LanguageManager.getLanguageForPath("/code/html")).toBe(unknown); expect(LanguageManager.getLanguageForPath("/code/foo.html.notreally")).toBe(unknown); }); From 98f23f2eaf5b5c7c01d4a0f1f040f1de963e5f42 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Mon, 29 Jul 2013 20:32:27 -0700 Subject: [PATCH 4/5] Code review fixes - tweak docs, cleanup vars, convert for to forEach() --- src/document/DocumentManager.js | 14 ++++++-------- src/widgets/Dialogs.js | 6 ++---- test/spec/FileIndexManager-test.js | 22 ++++++++++++---------- test/spec/SpecRunnerUtils.js | 9 +++++++-- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 37f8a23c361..867dd54de04 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -817,8 +817,6 @@ define(function (require, exports, module) { * @param {boolean} isFolder True if path is a folder; False if it is a file. */ function notifyPathNameChanged(oldName, newName, isFolder) { - var i; - // Update open documents. This will update _currentDocument too, since // the current document is always open. var keysToDelete = []; @@ -837,14 +835,14 @@ define(function (require, exports, module) { }); // Delete the old keys - for (i = 0; i < keysToDelete.length; i++) { - delete _openDocuments[keysToDelete[i]]; - } + keysToDelete.forEach(function (fullPath) { + delete _openDocuments[fullPath]; + }); // Update working set - for (i = 0; i < _workingSet.length; i++) { - FileUtils.updateFileEntryPath(_workingSet[i], oldName, newName, isFolder); - } + _workingSet.forEach(function (fileEntry) { + FileUtils.updateFileEntryPath(fileEntry, oldName, newName, isFolder); + }); // Send a "fileNameChanged" event. This will trigger the views to update. $(exports).triggerHandler("fileNameChange", [oldName, newName]); diff --git a/src/widgets/Dialogs.js b/src/widgets/Dialogs.js index fa9d30d1ba8..4533c0b146c 100644 --- a/src/widgets/Dialogs.js +++ b/src/widgets/Dialogs.js @@ -254,10 +254,8 @@ define(function (require, exports, module) { * as parameters as described. * * @param {string} dlgClass A class name identifier for the dialog. Typically one of DefaultDialogs.* - * @param {string=} title The title of the dialog. Can contain HTML markup. If unspecified, the title - * in the JSON file is used unchanged. - * @param {string=} message The message to display in the dialog. Can contain HTML markup. If - * unspecified, the message in the JSON file is used. + * @param {string=} title The title of the dialog. Can contain HTML markup. Defaults to "". + * @param {string=} message The message to display in the dialog. Can contain HTML markup. Defaults to "". * @param {Array.<{className: string, id: string, text: string>=} buttons An array of buttons where each button * has a class, id and text property. The id is used in "data-button-id". Defaults to a single Ok button. * Typically className is one of DIALOG_BTN_CLASS_*, id is one of DIALOG_BTN_* diff --git a/test/spec/FileIndexManager-test.js b/test/spec/FileIndexManager-test.js index 7d4efd2f338..96f85c2b1ca 100644 --- a/test/spec/FileIndexManager-test.js +++ b/test/spec/FileIndexManager-test.js @@ -248,22 +248,23 @@ define(function (require, exports, module) { }); it("should handle identical simultaneous requests without doing extra work", function () { // #330 + var projectRoot, + allFiles1, + allFiles2; + // Open a directory SpecRunnerUtils.loadProjectInTestWindow(testPath); - var projectRoot; - var promise1, promise2; - var allFiles1, allFiles2; runs(function () { projectRoot = ProjectManager.getProjectRoot(); spyOn(projectRoot, "createReader").andCallThrough(); // Kick off two index requests in parallel - promise1 = FileIndexManager.getFileInfoList("all") + var promise1 = FileIndexManager.getFileInfoList("all") .done(function (result) { allFiles1 = result; }); - promise2 = FileIndexManager.getFileInfoList("all") // same request again + var promise2 = FileIndexManager.getFileInfoList("all") // same request again .done(function (result) { allFiles2 = result; }); @@ -283,22 +284,23 @@ define(function (require, exports, module) { }); it("should handle differing simultaneous requests without doing extra work", function () { // #330 + var projectRoot, + allFiles1, + allFiles2; + // Open a directory SpecRunnerUtils.loadProjectInTestWindow(testPath); - var projectRoot; - var promise1, promise2; - var allFiles1, allFiles2; runs(function () { projectRoot = ProjectManager.getProjectRoot(); spyOn(projectRoot, "createReader").andCallThrough(); // Kick off two index requests in parallel - promise1 = FileIndexManager.getFileInfoList("all") + var promise1 = FileIndexManager.getFileInfoList("all") .done(function (result) { allFiles1 = result; }); - promise2 = FileIndexManager.getFileInfoList("css") // different request in parallel + var promise2 = FileIndexManager.getFileInfoList("css") // different request in parallel .done(function (result) { allFiles2 = result; }); diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index 974d822821f..40ec637e265 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -203,8 +203,8 @@ define(function (require, exports, module) { * * Unlike a real Document, does NOT need to be explicitly cleaned up. * - * @param {?string} initialContent Defaults to "" - * @param {?string} languageId Defaults to JavaScript + * @param {string=} initialContent Defaults to "" + * @param {string=} languageId Defaults to JavaScript */ function createMockDocument(initialContent, languageId) { var language = LanguageManager.getLanguage(languageId) || LanguageManager.getLanguage("javascript"), @@ -248,6 +248,8 @@ define(function (require, exports, module) { * * Must be cleaned up by calling destroyMockEditor(document) later. * + * @param {!Document} doc + * @param {{startLine: number, endLine: number}=} visibleRange * @return {!Editor} */ function createMockEditorForDocument(doc, visibleRange) { @@ -272,6 +274,9 @@ define(function (require, exports, module) { * * Must be cleaned up by calling destroyMockEditor(document) later. * + * @param {string=} initialContent + * @param {string=} languageId + * @param {{startLine: number, endLine: number}=} visibleRange * @return {!{doc:!Document, editor:!Editor}} */ function createMockEditor(initialContent, languageId, visibleRange) { From 9df783096178a098075753e83c3fe2db349a4564 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Tue, 30 Jul 2013 16:33:50 -0700 Subject: [PATCH 5/5] Code review: some more small JSDoc fixes --- src/language/LanguageManager.js | 12 ++++++------ src/widgets/Dialogs.js | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 5c7bb610b36..1f6b4ca5082 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -373,9 +373,9 @@ define(function (require, exports, module) { /** * Loads a mode and sets it for this language. * - * @param {string|Array.} mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] - * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. - * + * @param {(string|Array.)} mode CodeMirror mode (e.g. "htmlmixed"), optionally paired with a MIME mode defined by + * that mode (e.g. ["clike", "text/x-c++src"]). Unless the mode is located in thirdparty/CodeMirror2/mode//.js, + * you need to first load it yourself. * @return {$.Promise} A promise object that will be resolved when the mode is loaded and set */ Language.prototype._loadAndSetMode = function (mode) { @@ -518,7 +518,7 @@ define(function (require, exports, module) { /** * Sets the prefixes to use for line comments in this language or prints an error to the console. - * @param {!string|Array.} prefix Prefix string or an array of prefix strings + * @param {!(string|Array.)} prefix Prefix string or an array of prefix strings * to use for line comments (e.g. "//" or ["//", "#"]) * @return {boolean} Whether the syntax was valid and set or not */ @@ -643,8 +643,8 @@ define(function (require, exports, module) { * @param {Array.} definition.fileExtensions List of file extensions used by this language (e.g. ["php", "php3"] or ["coffee.md"] - may contain dots) * @param {Array.} definition.fileNames List of exact file names (e.g. ["Makefile"] or ["package.json]). Higher precedence than file extension. * @param {Array.} definition.blockComment Array with two entries defining the block comment prefix and suffix (e.g. [""]) - * @param {string|Array.} definition.lineComment Line comment prefixes (e.g. "//" or ["//", "#"]) - * @param {string|Array.} definition.mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] + * @param {(string|Array.)} definition.lineComment Line comment prefixes (e.g. "//" or ["//", "#"]) + * @param {(string|Array.)} definition.mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. * * @return {$.Promise} A promise object that will be resolved with a Language object diff --git a/src/widgets/Dialogs.js b/src/widgets/Dialogs.js index 4533c0b146c..075795e7f3f 100644 --- a/src/widgets/Dialogs.js +++ b/src/widgets/Dialogs.js @@ -256,7 +256,7 @@ define(function (require, exports, module) { * @param {string} dlgClass A class name identifier for the dialog. Typically one of DefaultDialogs.* * @param {string=} title The title of the dialog. Can contain HTML markup. Defaults to "". * @param {string=} message The message to display in the dialog. Can contain HTML markup. Defaults to "". - * @param {Array.<{className: string, id: string, text: string>=} buttons An array of buttons where each button + * @param {Array.<{className: string, id: string, text: string}>=} buttons An array of buttons where each button * has a class, id and text property. The id is used in "data-button-id". Defaults to a single Ok button. * Typically className is one of DIALOG_BTN_CLASS_*, id is one of DIALOG_BTN_* * @return {Dialog}