From a62e2b1a358dcc0955f032d77dc97ac174e4b1cc Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Sat, 5 Apr 2014 13:12:14 +0100 Subject: [PATCH 1/7] fix outstanding code review issues --- .../default/CSSShapesEditor/LiveEditorLocalDriver.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js index ae248bb5d0d..1ea14037f38 100644 --- a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js +++ b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js @@ -86,7 +86,6 @@ define(function (require, exports, module) { } /** - * @private * Inject remote live editor driver and any specified editor providers. * The remote live editor driver mirrors most of the local live editor driver API * to provide an interface to the in-browser live editor. @@ -158,7 +157,7 @@ define(function (require, exports, module) { * Promises can fail if the user manually refreshes the page or navigates * because the injected editor files will be lost. * - * @param {$.Promise=} result promise result + * @param {$.Deferred=} result */ function _whenRemoteCallFailed(result) { if (result) { From 52c1ca02c27c45f130aa6d7ccf1521ed6c11103e Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Sat, 5 Apr 2014 13:14:17 +0100 Subject: [PATCH 2/7] Fixes #7418 and #7421 - attempts to recover or teardown editor when encoutering errors --- .../CSSShapesEditor/LiveEditorLocalDriver.js | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js index 1ea14037f38..182140d798d 100644 --- a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js +++ b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js @@ -103,16 +103,16 @@ define(function (require, exports, module) { } /** - * @private * Send instructions to remove the live editor from the page in LivePreview. * @return {$.Promise} */ function remove() { if (_hasEditor === false) { var deferred = $.Deferred(); - return deferred.reject(); + return deferred.reject().promise(); } + _cache.model = undefined; // do not move in _reset(), otherwhise the _reconnect() scenario miss the cache and fail _reset(); var expr = _namespace + ".remove()"; return _call(expr); @@ -137,6 +137,10 @@ define(function (require, exports, module) { hasChanged = false, key; + if (!data) { + remove(); + } + // sync the local model snapshot with the remote model _.forEach(data, function (value, key) { if (!_model[key] || !_.isEqual(_model[key], value)) { @@ -154,18 +158,26 @@ define(function (require, exports, module) { /** * @private * Handle failed promises for eval() calls to the inspected page. - * Promises can fail if the user manually refreshes the page or navigates + * Promises fail if the user manually refreshes the page or navigates * because the injected editor files will be lost. + * If this is the case, attempt to reconnect. + * + * Promises also fail because of errors thrown in the remote page. + * If this is the case, remove the editor. * * @param {$.Deferred=} result */ function _whenRemoteCallFailed(result) { - if (result) { - return _reconnect(); - } else { - _cache.model = undefined; - return remove(); - } + // check if the remote editor namespace is still defined on the page + _call(_namespace) + .then(function(response) { + if (response.type === "undefined") { + _reconnect(); + } else { + remove(); + } + }) + .fail(remove); } /** From a17a8023b15dada76265933303dbb99b268b8dc8 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Sat, 5 Apr 2014 13:17:22 +0100 Subject: [PATCH 3/7] add a space to please the linter --- src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js index 182140d798d..a01b9f8f306 100644 --- a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js +++ b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js @@ -170,7 +170,7 @@ define(function (require, exports, module) { function _whenRemoteCallFailed(result) { // check if the remote editor namespace is still defined on the page _call(_namespace) - .then(function(response) { + .then(function (response) { if (response.type === "undefined") { _reconnect(); } else { From 89adb73f008e634c2308e6f8fbf7c36ee78c2989 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Sat, 5 Apr 2014 17:24:28 +0100 Subject: [PATCH 4/7] fix typos --- src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js index a01b9f8f306..e177c722885 100644 --- a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js +++ b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js @@ -112,7 +112,7 @@ define(function (require, exports, module) { return deferred.reject().promise(); } - _cache.model = undefined; // do not move in _reset(), otherwhise the _reconnect() scenario miss the cache and fail + _cache.model = undefined; // do not move in _reset(), otherwhise the _reconnect() scenario misses the cache and fails _reset(); var expr = _namespace + ".remove()"; return _call(expr); From 700609d97d5876e0d61220c3315b56cb83c3caf1 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Sat, 5 Apr 2014 17:25:57 +0100 Subject: [PATCH 5/7] remove duplicate unit test --- src/extensions/default/CSSShapesEditor/unittests.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/extensions/default/CSSShapesEditor/unittests.js b/src/extensions/default/CSSShapesEditor/unittests.js index 009423245d8..2eccf019075 100644 --- a/src/extensions/default/CSSShapesEditor/unittests.js +++ b/src/extensions/default/CSSShapesEditor/unittests.js @@ -466,17 +466,6 @@ define(function (require, exports, module) { expect(LiveEditorLocalDriver.remove).toHaveBeenCalled(); }); - - it("should call remove() when LivePreview is turned off", function () { - var deferred = $.Deferred(); - spyOn(LiveEditorLocalDriver, "remove").andReturn(deferred.promise()); - - main._setup(); - main._teardown(); - - expect(LiveEditorLocalDriver.remove).toHaveBeenCalled(); - }); - }); describe("Live Preview Workflow", function () { From a301e8ca31a2a7ff58eddbbcf3f9a8271a6fdf43 Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Mon, 7 Apr 2014 11:13:21 +0100 Subject: [PATCH 6/7] revert to using only T key as a trigger; Ctrl+T triggers a new tab on Windows --- .../CSSShapesEditor/thirdparty/CSSShapesEditorProvider.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/extensions/default/CSSShapesEditor/thirdparty/CSSShapesEditorProvider.js b/src/extensions/default/CSSShapesEditor/thirdparty/CSSShapesEditorProvider.js index 15793a6cbd8..490d5bbd729 100644 --- a/src/extensions/default/CSSShapesEditor/thirdparty/CSSShapesEditorProvider.js +++ b/src/extensions/default/CSSShapesEditor/thirdparty/CSSShapesEditorProvider.js @@ -82,7 +82,7 @@ Declared globally within module so it can be removed by Provider.remove() Defined here so it can access scope.inst - Ctrl+T toggles the free transform editor (scale/rotate) + T key toggles the free transform editor (scale/rotate) Esc key turns off free transform editor; quietly ignored if editor was never turned on. @param {Event} e keydown event @@ -92,8 +92,8 @@ if (scope.inst.type !== "polygon") { return; } - // Ctrl+T toggles rotate/scale editor - if (e.ctrlKey && e.keyIdentifier === "U+0054") { + // T key toggles rotate/scale editor + if (e.keyIdentifier === "U+0054") { scope.inst.toggleFreeTransform(); } From 5d6cfe912689afa15772163fdac9bb79d87617aa Mon Sep 17 00:00:00 2001 From: Razvan Caliman Date: Mon, 7 Apr 2014 11:22:05 +0100 Subject: [PATCH 7/7] fix typo; undocument unused param --- .../default/CSSShapesEditor/LiveEditorLocalDriver.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js index e177c722885..a1ec92a29bc 100644 --- a/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js +++ b/src/extensions/default/CSSShapesEditor/LiveEditorLocalDriver.js @@ -112,7 +112,7 @@ define(function (require, exports, module) { return deferred.reject().promise(); } - _cache.model = undefined; // do not move in _reset(), otherwhise the _reconnect() scenario misses the cache and fails + _cache.model = undefined; // do not move in _reset(), otherwise the _reconnect() scenario misses the cache and fails _reset(); var expr = _namespace + ".remove()"; return _call(expr); @@ -164,10 +164,8 @@ define(function (require, exports, module) { * * Promises also fail because of errors thrown in the remote page. * If this is the case, remove the editor. - * - * @param {$.Deferred=} result */ - function _whenRemoteCallFailed(result) { + function _whenRemoteCallFailed() { // check if the remote editor namespace is still defined on the page _call(_namespace) .then(function (response) {