From cf686282ef69ba75ca4a9f9226fcbdd5a0698076 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 11 May 2017 12:26:14 -0300 Subject: [PATCH 1/4] Do not use cookie for pad shortcuts Users still cannot choose which shortcuts they want to enable/disable, so it does not make sense (yet) to have a cookie with that preference. This can be reverted once we create an UI to change shortcuts, but PLEASE PLEASE PLEASE do not read the cookie every time handleKeyEvent is called!!! This is an adjustment to #2891. --- src/node/handler/PadMessageHandler.js | 1 + src/node/hooks/express/specialpages.js | 6 ----- src/static/js/ace2_inner.js | 33 +------------------------- 3 files changed, 2 insertions(+), 38 deletions(-) diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 20b262f4..b7ec7cb2 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -1197,6 +1197,7 @@ function handleClientReady(client, message) "userColor": authorColorId, "padId": message.padId, "padOptions": settings.padOptions, + "padShortcutEnabled": settings.padShortcutEnabled, "initialTitle": "Pad: " + message.padId, "opts": {}, // tell the client the number of the latest chat-message, which will be diff --git a/src/node/hooks/express/specialpages.js b/src/node/hooks/express/specialpages.js index e933a05a..2840f82c 100644 --- a/src/node/hooks/express/specialpages.js +++ b/src/node/hooks/express/specialpages.js @@ -48,12 +48,6 @@ exports.expressCreateServer = function (hook_name, args, cb) { res.cookie('language', settings.padOptions.lang); } - // Enable the pad shortcut keys from settings.json - if (settings.padShortcutEnabled !== undefined) - { - res.cookie('padShortcutEnabled', JSON.stringify(settings.padShortcutEnabled)); - } - // The below might break for pads being rewritten var isReadOnly = req.url.indexOf("/p/r.") === 0; diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index b6d4bd25..3b103988 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -61,7 +61,6 @@ function Ace2Inner(){ var SkipList = require('./skiplist'); var undoModule = require('./undomodule').undoModule; var AttributeManager = require('./AttributeManager'); - var readCookie = require('./pad_utils').readCookie; var DEBUG = false; //$$ build script replaces the string "var DEBUG=true;//$$" with "var DEBUG=false;" // changed to false @@ -3641,37 +3640,6 @@ function Ace2Inner(){ function handleKeyEvent(evt) { - // Get the enabled shortcut keys - // If it can't find the cookie, use default values - // Cookie should normally be set - // See settings.json - var padShortcutEnabled = JSON.parse(decodeURIComponent(readCookie('padShortcutEnabled'))); - if (!padShortcutEnabled) - { - padShortcutEnabled = { - "altF9" : true, - "altC" : true, - "cmdShift2" : true, - "delete" : true, - "return" : true, - "cmdS" : true, - "tab" : true, - "cmdZ" : true, - "cmdY" : true, - "cmdI" : true, - "cmdB" : true, - "cmdU" : true, - "cmd5" : true, - "cmdShiftL" : true, - "cmdShiftN" : true, - "cmdShiftC" : true, - "cmdH" : true, - "ctrlHome" : true, - "pageUp" : true, - "pageDown" : true, - } - } - // if (DEBUG && window.DONT_INCORP) return; if (!isEditable) return; var type = evt.type; @@ -3755,6 +3723,7 @@ function Ace2Inner(){ specialHandled = _.contains(specialHandledInHook, true); } + var padShortcutEnabled = parent.parent.clientVars.padShortcutEnabled; if ((!specialHandled) && altKey && isTypeForSpecialKey && keyCode == 120 && padShortcutEnabled.altF9){ // Alt F9 focuses on the File Menu and/or editbar. // Note that while most editors use Alt F10 this is not desirable From 688e8f37a3a9557f5cec13692d97da01bfa19bf7 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 11 May 2017 12:30:36 -0300 Subject: [PATCH 2/4] [fix] Fix format of settings template + add information about shortcuts There was an extra comma at the end of shortcut list, this was breaking Etherpad startup. This is an adjustment to #2891. --- settings.json.template | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/settings.json.template b/settings.json.template index 798ce0ef..e89f4303 100644 --- a/settings.json.template +++ b/settings.json.template @@ -75,26 +75,26 @@ /* Pad Shortcut Keys */ "padShortcutEnabled" : { - "altF9" : true, - "altC" : true, - "cmdShift2" : true, - "delete" : true, - "return" : true, - "cmdS" : true, - "tab" : true, - "cmdZ" : true, - "cmdY" : true, - "cmdI" : true, - "cmdB" : true, - "cmdU" : true, - "cmd5" : true, - "cmdShiftL" : true, - "cmdShiftN" : true, - "cmdShiftC" : true, - "cmdH" : true, - "ctrlHome" : true, - "pageUp" : true, - "pageDown" : true, + "altF9" : true, /* focus on the File Menu and/or editbar */ + "altC" : true, /* focus on the Chat window */ + "cmdShift2" : true, /* shows a gritter popup showing a line author */ + "delete" : true, + "return" : true, + "cmdS" : true, /* save a revision */ + "tab" : true, /* indent */ + "cmdZ" : true, /* undo/redo */ + "cmdY" : true, /* redo */ + "cmdI" : true, /* italic */ + "cmdB" : true, /* bold */ + "cmdU" : true, /* underline */ + "cmd5" : true, /* strike through */ + "cmdShiftL" : true, /* unordered list */ + "cmdShiftN" : true, /* ordered list */ + "cmdShiftC" : true, /* clear authorship */ + "cmdH" : true, /* backspace */ + "ctrlHome" : true, /* scroll to top of pad */ + "pageUp" : true, + "pageDown" : true }, /* Should we suppress errors from being visible in the default Pad Text? */ From 97038c2183c9f2c6f427825e121f841db01eb1c8 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 11 May 2017 12:35:25 -0300 Subject: [PATCH 3/4] [fix] Fix shortcut enabling flag for 'ESC' This is an adjustment to #2891. --- settings.json.template | 1 + src/node/utils/Settings.js | 1 + src/static/js/ace2_inner.js | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/settings.json.template b/settings.json.template index e89f4303..b0bbfbde 100644 --- a/settings.json.template +++ b/settings.json.template @@ -80,6 +80,7 @@ "cmdShift2" : true, /* shows a gritter popup showing a line author */ "delete" : true, "return" : true, + "esc" : true, /* in mozilla versions 14-19 avoid reconnecting pad */ "cmdS" : true, /* save a revision */ "tab" : true, /* indent */ "cmdZ" : true, /* undo/redo */ diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js index 5060d28f..e9b02449 100644 --- a/src/node/utils/Settings.js +++ b/src/node/utils/Settings.js @@ -111,6 +111,7 @@ exports.padShortcutEnabled = { "delete" : true, "cmdShift2" : true, "return" : true, + "esc" : true, "cmdS" : true, "tab" : true, "cmdZ" : true, diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 3b103988..a0030c62 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -3847,7 +3847,7 @@ function Ace2Inner(){ }, 0); specialHandled = true; } - if ((!specialHandled) && isTypeForSpecialKey && keyCode == 27 && padShortcutEnabled.cmdS) + if ((!specialHandled) && isTypeForSpecialKey && keyCode == 27 && padShortcutEnabled.esc) { // prevent esc key; // in mozilla versions 14-19 avoid reconnecting pad. From 0cb8d31e9569c3b80fb5847a664f28188f67a2ff Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 11 May 2017 14:56:09 -0300 Subject: [PATCH 4/4] [fix] Have one setting for each shortcut to create ordered list This is an adjustment to #2891. --- settings.json.template | 1 + src/node/utils/Settings.js | 1 + src/static/js/ace2_inner.js | 4 +- tests/frontend/specs/ordered_list.js | 98 ++++++++++++++++++++++++++-- 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/settings.json.template b/settings.json.template index b0bbfbde..0cb10d50 100644 --- a/settings.json.template +++ b/settings.json.template @@ -91,6 +91,7 @@ "cmd5" : true, /* strike through */ "cmdShiftL" : true, /* unordered list */ "cmdShiftN" : true, /* ordered list */ + "cmdShift1" : true, /* ordered list */ "cmdShiftC" : true, /* clear authorship */ "cmdH" : true, /* backspace */ "ctrlHome" : true, /* scroll to top of pad */ diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js index e9b02449..660b7afb 100644 --- a/src/node/utils/Settings.js +++ b/src/node/utils/Settings.js @@ -122,6 +122,7 @@ exports.padShortcutEnabled = { "cmd5" : true, "cmdShiftL" : true, "cmdShiftN" : true, + "cmdShift1" : true, "cmdShiftC" : true, "cmdH" : true, "ctrlHome" : true, diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index a0030c62..b3759e52 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -3939,9 +3939,9 @@ function Ace2Inner(){ doInsertUnorderedList() specialHandled = true; } - if ((!specialHandled) && isTypeForCmdKey && (String.fromCharCode(which).toLowerCase() == "n" || String.fromCharCode(which) == 1) && (evt.metaKey || evt.ctrlKey) && evt.shiftKey && padShortcutEnabled.cmdShiftN) + if ((!specialHandled) && isTypeForCmdKey && ((String.fromCharCode(which).toLowerCase() == "n" && padShortcutEnabled.cmdShiftN) || (String.fromCharCode(which) == 1 && padShortcutEnabled.cmdShift1)) && (evt.metaKey || evt.ctrlKey) && evt.shiftKey) { - // cmd-shift-N (orderedlist) + // cmd-shift-N and cmd-shift-1 (orderedlist) fastIncorp(9); evt.preventDefault(); doInsertOrderedList() diff --git a/tests/frontend/specs/ordered_list.js b/tests/frontend/specs/ordered_list.js index ca7d755e..57196fef 100644 --- a/tests/frontend/specs/ordered_list.js +++ b/tests/frontend/specs/ordered_list.js @@ -5,8 +5,8 @@ describe("assign ordered list", function(){ this.timeout(60000); }); - it("insert ordered list text", function(done){ - var inner$ = helper.padInner$; + it("inserts ordered list text", function(done){ + var inner$ = helper.padInner$; var chrome$ = helper.padChrome$; var $insertorderedlistButton = chrome$(".buttonicon-insertorderedlist"); @@ -17,8 +17,72 @@ describe("assign ordered list", function(){ }).done(done); }); + context('when user presses Ctrl+Shift+N', function() { + context('and pad shortcut is enabled', function() { + beforeEach(function() { + makeSureShortcutIsEnabled('cmdShiftN'); + triggerCtrlShiftShortcut('N'); + }); + + it('inserts unordered list', function(done) { + helper.waitFor(function() { + return helper.padInner$('div').first().find('ol li').length === 1; + }).done(done); + }); + }); + + context('and pad shortcut is disabled', function() { + beforeEach(function() { + makeSureShortcutIsDisabled('cmdShiftN'); + triggerCtrlShiftShortcut('N'); + }); + + it('does not insert unordered list', function(done) { + helper.waitFor(function() { + return helper.padInner$('div').first().find('ol li').length === 1; + }).done(function() { + expect().fail(function() { return 'Unordered list inserted, should ignore shortcut' }); + }).fail(function() { + done(); + }); + }); + }); + }); + + context('when user presses Ctrl+Shift+1', function() { + context('and pad shortcut is enabled', function() { + beforeEach(function() { + makeSureShortcutIsEnabled('cmdShift1'); + triggerCtrlShiftShortcut('1'); + }); + + it('inserts unordered list', function(done) { + helper.waitFor(function() { + return helper.padInner$('div').first().find('ol li').length === 1; + }).done(done); + }); + }); + + context('and pad shortcut is disabled', function() { + beforeEach(function() { + makeSureShortcutIsDisabled('cmdShift1'); + triggerCtrlShiftShortcut('1'); + }); + + it('does not insert unordered list', function(done) { + helper.waitFor(function() { + return helper.padInner$('div').first().find('ol li').length === 1; + }).done(function() { + expect().fail(function() { return 'Unordered list inserted, should ignore shortcut' }); + }).fail(function() { + done(); + }); + }); + }); + }); + xit("issue #1125 keeps the numbered list on enter for the new line - EMULATES PASTING INTO A PAD", function(done){ - var inner$ = helper.padInner$; + var inner$ = helper.padInner$; var chrome$ = helper.padChrome$; var $insertorderedlistButton = chrome$(".buttonicon-insertorderedlist"); @@ -26,9 +90,9 @@ describe("assign ordered list", function(){ //type a bit, make a line break and type again var $firstTextElement = inner$("div span").first(); - $firstTextElement.sendkeys('line 1'); - $firstTextElement.sendkeys('{enter}'); - $firstTextElement.sendkeys('line 2'); + $firstTextElement.sendkeys('line 1'); + $firstTextElement.sendkeys('{enter}'); + $firstTextElement.sendkeys('line 2'); $firstTextElement.sendkeys('{enter}'); helper.waitFor(function(){ @@ -44,4 +108,26 @@ describe("assign ordered list", function(){ done(); }); }); + + var triggerCtrlShiftShortcut = function(shortcutChar) { + var inner$ = helper.padInner$; + if(inner$(window)[0].bowser.firefox || inner$(window)[0].bowser.modernIE) { // if it's a mozilla or IE + var evtType = "keypress"; + }else{ + var evtType = "keydown"; + } + var e = inner$.Event(evtType); + e.ctrlKey = true; + e.shiftKey = true; + e.which = shortcutChar.toString().charCodeAt(0); + inner$("#innerdocbody").trigger(e); + } + + var makeSureShortcutIsDisabled = function(shortcut) { + helper.padChrome$.window.clientVars.padShortcutEnabled[shortcut] = false; + } + var makeSureShortcutIsEnabled = function(shortcut) { + helper.padChrome$.window.clientVars.padShortcutEnabled[shortcut] = true; + } + });