From 5fd5f4318a3aa7f2e0b7ea36df6654ebff243a01 Mon Sep 17 00:00:00 2001
From: Martin Edenhofer <me@edenhofer.de>
Date: Mon, 10 Apr 2017 08:08:18 +0200
Subject: [PATCH] Fixed issue #656 - Shortcuts in different keyboard layout.

---
 .../widget/keyboard_shortcuts.coffee          |  15 +-
 .../widget/translation_inline.coffee          |  12 +-
 .../app/lib/app_post/browser.coffee.coffee    |  14 ++
 .../app/lib/base/jquery.contenteditable.js    | 163 +++++++++++-------
 .../app/views/keyboard_shortcuts.jst.eco      |   6 +-
 test/browser/admin_role_test.rb               | 104 ++++++++++-
 test/browser/translation_test.rb              |  16 +-
 test/browser_test_helper.rb                   |  54 +++---
 8 files changed, 279 insertions(+), 105 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/widget/keyboard_shortcuts.coffee b/app/assets/javascripts/app/controllers/widget/keyboard_shortcuts.coffee
index 328c1138c..65601118d 100644
--- a/app/assets/javascripts/app/controllers/widget/keyboard_shortcuts.coffee
+++ b/app/assets/javascripts/app/controllers/widget/keyboard_shortcuts.coffee
@@ -13,6 +13,8 @@ class App.KeyboardShortcutModal extends App.ControllerModal
   content: ->
     App.view('keyboard_shortcuts')(
       areas: App.Config.get('keyboard_shortcuts')
+      magicKey: App.Browser.magicKey()
+      hotkeys: App.Browser.hotkeys().split('+').reverse()
     )
 
   exists: =>
@@ -36,7 +38,8 @@ class App.KeyboardShortcutWidget extends Spine.Module
     )
 
   observerKeys: =>
-    navigationHotkeys = 'alt+ctrl'
+    navigationHotkeys = App.Browser.hotkeys()
+
     areas = App.Config.get('keyboard_shortcuts')
     for area in areas
       for item in area.content
@@ -401,25 +404,25 @@ App.Config.set(
           shortcuts: [
             {
               key: 'u'
-              hotkeys: true
+              magicKey: true
               description: 'Format as _underlined_'
               globalEvent: 'richtext-underline'
             }
             {
               key: 'b'
-              hotkeys: true
+              magicKey: true
               description: 'Format as |bold|'
               globalEvent: 'richtext-bold'
             }
             {
               key: 'i'
-              hotkeys: true
+              magicKey: true
               description: 'Format as ||italic||'
               globalEvent: 'richtext-italic'
             }
             {
-              key: 'v'
-              hotkeys: true
+              key: 's'
+              magicKey: true
               description: 'Format as //strikethrough//'
               globalEvent: 'richtext-strikethrough'
             }
diff --git a/app/assets/javascripts/app/controllers/widget/translation_inline.coffee b/app/assets/javascripts/app/controllers/widget/translation_inline.coffee
index 474871faf..4a27407fd 100644
--- a/app/assets/javascripts/app/controllers/widget/translation_inline.coffee
+++ b/app/assets/javascripts/app/controllers/widget/translation_inline.coffee
@@ -12,9 +12,17 @@ class Widget extends App.Controller
     return if !@permissionCheck('admin.translation')
 
     # bind on key down
-    # if ctrl+alt+t is pressed, enable translation_inline and fire ui:rerender
+    # if hotkeys+t is pressed, enable translation_inline and fire ui:rerender
+    browserHotkeys = App.Browser.hotkeys()
     $(document).on('keydown.translation', (e) =>
-      if e.altKey && e.ctrlKey && e.keyCode is 84
+      hotkeys = false
+      if browserHotkeys is 'ctrl+shift'
+        if !e.altKey && e.ctrlKey && !e.metaKey && e.shiftKey
+          hotkeys = true
+      else
+        if e.altKey && e.ctrlKey && !e.metaKey
+          hotkeys = true
+      if hotkeys && e.keyCode is 84
         @toogle()
     )
 
diff --git a/app/assets/javascripts/app/lib/app_post/browser.coffee.coffee b/app/assets/javascripts/app/lib/app_post/browser.coffee.coffee
index 35fb62680..53bf5f5fa 100644
--- a/app/assets/javascripts/app/lib/app_post/browser.coffee.coffee
+++ b/app/assets/javascripts/app/lib/app_post/browser.coffee.coffee
@@ -82,6 +82,20 @@ class App.Browser
       localStorage.setItem('fingerprint', fingerprint)
     fingerprint
 
+  @magicKey: ->
+    browser = @detection()
+    magicKey = 'ctrl'
+    if browser && browser.os && browser.os.name.toString().match(/mac/i)
+      magicKey = 'cmd'
+    magicKey
+
+  @hotkeys: ->
+    browser = @detection()
+    hotkeys = 'ctrl+shift'
+    if browser && browser.os && browser.os.name.toString().match(/mac/i)
+      hotkeys = 'alt+ctrl'
+    hotkeys
+
 class Modal extends App.ControllerModal
   buttonClose: false
   buttonCancel: false
diff --git a/app/assets/javascripts/app/lib/base/jquery.contenteditable.js b/app/assets/javascripts/app/lib/base/jquery.contenteditable.js
index bddbcde12..606402b01 100644
--- a/app/assets/javascripts/app/lib/base/jquery.contenteditable.js
+++ b/app/assets/javascripts/app/lib/base/jquery.contenteditable.js
@@ -39,6 +39,7 @@
       66: true, // b
       73: true, // i
       85: true, // u
+      83: true, // s
     },
     //maxlength: 20,
   };
@@ -59,11 +60,9 @@
 
     this.preventInput = false
 
-    // detect firefox / handle contenteditable issues
-    this.browser = undefined
-    if ( navigator && navigator.userAgent && navigator.userAgent.search('Firefox') != -1) {
-      this.browser = 'ff'
-    }
+    // handle contenteditable issues
+    this.browserMagicKey = App.Browser.magicKey()
+    this.browserHotkeys = App.Browser.hotkeys()
 
     this.init();
   }
@@ -112,28 +111,76 @@
         }
       }
 
-      // on zammad altKey + ctrlKey + i/b/u
-      //  altKey + ctrlKey + u -> Toggles the current selection between underlined and not underlined
-      //  altKey + ctrlKey + b -> Toggles the current selection between bold and non-bold
-      //  altKey + ctrlKey + i -> Toggles the current selection between italic and non-italic
-      //  altKey + ctrlKey + v -> Toggles the current selection between strike and non-strike
-      //  altKey + ctrlKey + f -> Removes the formatting tags from the current selection
-      //  altKey + ctrlKey + y -> Removes the formatting from while textarea
-      //  altKey + ctrlKey + z -> Inserts a Horizontal Rule
-      //  altKey + ctrlKey + l -> Toggles the text selection between an unordered list and a normal block
-      //  altKey + ctrlKey + k -> Toggles the text selection between an ordered list and a normal block
-      //  altKey + ctrlKey + o -> Draws a line through the middle of the current selection
-      //  altKey + ctrlKey + w -> Removes any hyperlink from the current selection
-      if ( e.altKey && e.ctrlKey && !e.metaKey && (_this.options.richTextFormatKey[ e.keyCode ]
+      // on zammad magicKey + i/b/u/s
+      //  hotkeys + u -> Toggles the current selection between underlined and not underlined
+      //  hotkeys + b -> Toggles the current selection between bold and non-bold
+      //  hotkeys + i -> Toggles the current selection between italic and non-italic
+      //  hotkeys + v -> Toggles the current selection between strike and non-strike
+      //  hotkeys + f -> Removes the formatting tags from the current selection
+      //  hotkeys + y -> Removes the formatting from while textarea
+      //  hotkeys + z -> Inserts a Horizontal Rule
+      //  hotkeys + l -> Toggles the text selection between an unordered list and a normal block
+      //  hotkeys + k -> Toggles the text selection between an ordered list and a normal block
+      //  hotkeys + o -> Draws a line through the middle of the current selection
+      //  hotkeys + w -> Removes any hyperlink from the current selection
+      var richtTextControl = false
+      if (_this.browserMagicKey == 'cmd') {
+        if (!e.altKey && !e.ctrlKey && e.metaKey) {
+          richtTextControl = true
+        }
+      }
+      else {
+        if (!e.altKey && e.ctrlKey && !e.metaKey) {
+          richtTextControl = true
+        }
+      }
+      if (richtTextControl && _this.options.richTextFormatKey[ e.keyCode ]) {
+        e.preventDefault()
+        if (e.keyCode == 66) {
+          document.execCommand('bold')
+          return true
+        }
+        if (e.keyCode == 73) {
+          document.execCommand('italic')
+          return true
+        }
+        if (e.keyCode == 85) {
+          document.execCommand('underline')
+          return true
+        }
+        if (e.keyCode == 83) {
+          document.execCommand('strikeThrough')
+          return true
+        }
+      }
+
+      var hotkeys = false
+      if (_this.browserHotkeys == 'ctrl+shift') {
+        if (!e.altKey && e.ctrlKey && !e.metaKey && e.shiftKey) {
+          hotkeys = true
+        }
+      }
+      else {
+        if (e.altKey && e.ctrlKey && !e.metaKey) {
+          hotkeys = true
+        }
+      }
+
+      if (hotkeys && (_this.options.richTextFormatKey[ e.keyCode ]
         || e.keyCode == 49
         || e.keyCode == 50
         || e.keyCode == 51
+        || e.keyCode == 66
         || e.keyCode == 70
         || e.keyCode == 90
-        || e.keyCode == 76
+        || e.keyCode == 70
+        || e.keyCode == 73
         || e.keyCode == 75
+        || e.keyCode == 76
+        || e.keyCode == 85
         || e.keyCode == 86
         || e.keyCode == 87
+        || e.keyCode == 90
         || e.keyCode == 89)) {
         e.preventDefault()
 
@@ -142,37 +189,6 @@
           return
         }
 
-        if (e.keyCode == 66) {
-          document.execCommand('bold')
-        }
-        if (e.keyCode == 73) {
-          document.execCommand('italic')
-        }
-        if (e.keyCode == 85) {
-          document.execCommand('underline')
-        }
-        if (e.keyCode == 70) {
-          document.execCommand('removeFormat')
-        }
-        if (e.keyCode == 89) {
-          var cleanHtml = App.Utils.htmlRemoveRichtext(_this.$element.html())
-          _this.$element.html(cleanHtml)
-        }
-        if (e.keyCode == 90) {
-          document.execCommand('insertHorizontalRule')
-        }
-        if (e.keyCode == 76) {
-          document.execCommand('insertUnorderedList')
-        }
-        if (e.keyCode == 75) {
-          document.execCommand('insertOrderedList')
-        }
-        if (e.keyCode == 86) {
-          document.execCommand('strikeThrough')
-        }
-        if (e.keyCode == 87) {
-          document.execCommand('unlink')
-        }
         if (e.keyCode == 49) {
           _this.toggleBlock('h1')
         }
@@ -182,6 +198,37 @@
         if (e.keyCode == 51) {
           _this.toggleBlock('h3')
         }
+        if (e.keyCode == 66) {
+          document.execCommand('bold')
+        }
+        if (e.keyCode == 70) {
+          document.execCommand('removeFormat')
+        }
+        if (e.keyCode == 73) {
+          document.execCommand('italic')
+        }
+        if (e.keyCode == 75) {
+          document.execCommand('insertOrderedList')
+        }
+        if (e.keyCode == 76) {
+          document.execCommand('insertUnorderedList')
+        }
+        if (e.keyCode == 85) {
+          document.execCommand('underline')
+        }
+        if (e.keyCode == 86) {
+          document.execCommand('strikeThrough')
+        }
+        if (e.keyCode == 87) {
+          document.execCommand('unlink')
+        }
+        if (e.keyCode == 89) {
+          var cleanHtml = App.Utils.htmlRemoveRichtext(_this.$element.html())
+          _this.$element.html(cleanHtml)
+        }
+        if (e.keyCode == 90) {
+          document.execCommand('insertHorizontalRule')
+        }
         _this.log('content editable richtext key', e.keyCode)
         return true
       }
@@ -391,22 +438,6 @@
     return false
   }
 
-  // check if rich text key is pressed
-  Plugin.prototype.richTextKey = function(e) {
-    // e.altKey
-    // e.ctrlKey
-    // e.metaKey
-    // on mac block e.metaKey + i/b/u
-    if ( !e.altKey && e.metaKey && this.options.richTextFormatKey[ e.keyCode ] ) {
-      return true
-    }
-    // on win block e.ctrlKey + i/b/u
-    if ( !e.altKey && e.ctrlKey && this.options.richTextFormatKey[ e.keyCode ] ) {
-      return true
-    }
-    return false
-  }
-
   // max length check
   Plugin.prototype.maxLengthOk = function(typeAhead) {
     if ( !this.options.maxlength ) {
diff --git a/app/assets/javascripts/app/views/keyboard_shortcuts.jst.eco b/app/assets/javascripts/app/views/keyboard_shortcuts.jst.eco
index b572d69ae..587689fa6 100644
--- a/app/assets/javascripts/app/views/keyboard_shortcuts.jst.eco
+++ b/app/assets/javascripts/app/views/keyboard_shortcuts.jst.eco
@@ -7,7 +7,7 @@
       <% if item.where: %><p><i><%- @T(item.where) %></i></p><% end %>
       <% for shortcut in item.shortcuts: %>
         <% if shortcut.hotkeys: %>
-          <kbd>ctrl</kbd> <kbd>alt</kbd>
+          <kbd><%- @hotkeys.join('</kbd> <kbd>') %></kbd>
         <% end %>
         <% if shortcut.keyPrefix: %>
           <%= shortcut.keyPrefix %>
@@ -33,7 +33,9 @@
       <% if item.where: %><p><i><%- @T(item.where) %></i></p><% end %>
       <% for shortcut in item.shortcuts: %>
         <% if shortcut.hotkeys: %>
-          <kbd>ctrl</kbd> <kbd>alt</kbd>
+          <kbd><%- @hotkeys.join('</kbd> <kbd>') %></kbd>
+        <% else if shortcut.magicKey: %>
+          <kbd><%= @magicKey %></kbd>
         <% end %>
         <% if shortcut.keyPrefix: %>
           <%= shortcut.keyPrefix %>
diff --git a/test/browser/admin_role_test.rb b/test/browser/admin_role_test.rb
index 982d5cb8f..5c34baffa 100644
--- a/test/browser/admin_role_test.rb
+++ b/test/browser/admin_role_test.rb
@@ -2,7 +2,7 @@
 require 'browser_test_helper'
 
 class AdminRoleTest < TestCase
-  def test_role
+  def test_role_device
     name = "some role #{rand(99_999_999)}"
 
     @browser = browser_instance
@@ -125,4 +125,106 @@ class AdminRoleTest < TestCase
     )
   end
 
+  def test_role_admin_user
+
+    @browser = browser_instance
+
+    login(
+      username: 'agent1@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    # check if admin exists
+    exists_not(css: '[href="#manage"]')
+    logout()
+
+    # add admin.user to agent role
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+    tasks_close_all()
+
+    role_edit(
+      data: {
+        name:   'Agent',
+        active: true,
+        permission: {
+          'admin.user' => true,
+          'chat.agent' => true,
+          'cti.agent' => true,
+          'ticket.agent' => true,
+          'user_preferences' => true,
+        },
+      }
+    )
+    logout()
+
+    # check if admin exists
+    login(
+      username: 'agent1@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+    tasks_close_all()
+
+    # create user
+    random = rand(999_999_999)
+    user_email = "admin.user.#{rand}@example.com"
+    user_create(
+      data: {
+        #login:    "some login #{random}",
+        firstname: "Admin.User Firstname #{random}",
+        lastname:  "Admin.User Lastname #{random}",
+        email:     user_email,
+        password:  'some-pass',
+      },
+    )
+
+    # create ticket for user
+    ticket_create(
+      data: {
+        customer: user_email,
+        group: 'Users',
+        title: 'some changes',
+        body: 'some body 123äöü - admin.user',
+      },
+    )
+
+    # revoke admin.user
+    logout()
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+    tasks_close_all()
+
+    role_edit(
+      data: {
+        name:   'Agent',
+        active: true,
+        permission: {
+          'admin.user' => false,
+          'chat.agent' => true,
+          'cti.agent' => true,
+          'ticket.agent' => true,
+          'user_preferences' => true,
+        },
+      }
+    )
+    logout()
+
+    login(
+      username: 'agent1@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    # check if admin exists
+    exists_not(css: '[href="#manage"]')
+
+  end
 end
diff --git a/test/browser/translation_test.rb b/test/browser/translation_test.rb
index 436cc5ede..c05041895 100644
--- a/test/browser/translation_test.rb
+++ b/test/browser/translation_test.rb
@@ -109,9 +109,9 @@ class TranslationTest < TestCase
     )
 
     @browser.action.key_down(:control)
-            .key_down(:alt)
+            .key_down(:shift)
             .send_keys('t')
-            .key_up(:alt)
+            .key_up(:shift)
             .key_up(:control)
             .perform
 
@@ -128,9 +128,9 @@ class TranslationTest < TestCase
     sleep 5
 
     @browser.action.key_down(:control)
-            .key_down(:alt)
+            .key_down(:shift)
             .send_keys('t')
-            .key_up(:alt)
+            .key_up(:shift)
             .key_up(:control)
             .perform
 
@@ -194,9 +194,9 @@ class TranslationTest < TestCase
     sleep 5
 
     @browser.action.key_down(:control)
-            .key_down(:alt)
+            .key_down(:shift)
             .send_keys('t')
-            .key_up(:alt)
+            .key_up(:shift)
             .key_up(:control)
             .perform
 
@@ -213,9 +213,9 @@ class TranslationTest < TestCase
     sleep 5
 
     @browser.action.key_down(:control)
-            .key_down(:alt)
+            .key_down(:shift)
             .send_keys('t')
-            .key_up(:alt)
+            .key_up(:shift)
             .key_up(:control)
             .perform
 
diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb
index c4f87d94c..45de4a2c2 100644
--- a/test/browser_test_helper.rb
+++ b/test/browser_test_helper.rb
@@ -1364,9 +1364,9 @@ wait untill text in selector disabppears
     instance = params[:browser] || @browser
     screenshot(browser: instance, comment: 'shortcut_before')
     instance.action.key_down(:control)
-            .key_down(:alt)
+            .key_down(:shift)
             .send_keys(params[:key])
-            .key_up(:alt)
+            .key_up(:shift)
             .key_up(:control)
             .perform
     screenshot(browser: instance, comment: 'shortcut_after')
@@ -2944,10 +2944,10 @@ wait untill text in selector disabppears
     data: {
       name: 'some role' + random,
       default_at_signup: false,
-      permission: [
-        'admin.group',
-        'preferences.password',
-      ],
+      permission: {
+        'admin.group' => true,
+        'preferences.password' => true,
+      },
       member:    [
         'some_user_login',
       ],
@@ -2994,11 +2994,18 @@ wait untill text in selector disabppears
     end
 
     if data.key?(:permission)
-      data[:permission].each { |permission_name|
-        check(
-          browser: instance,
-          css:     ".modal [data-permission-name=\"#{permission_name}\"]",
-        )
+      data[:permission].each { |permission_name, permission_value|
+        if permission_value == false
+          uncheck(
+            browser: instance,
+            css:     ".modal [data-permission-name=\"#{permission_name}\"]",
+          )
+        else
+          check(
+            browser: instance,
+            css:     ".modal [data-permission-name=\"#{permission_name}\"]",
+          )
+        end
       }
     end
 
@@ -3046,10 +3053,10 @@ wait untill text in selector disabppears
     data: {
       name: 'some role' + random,
       default_at_signup: false,
-      permission: [
-        'admin.group',
-        'preferences.password',
-      ],
+      permission: {
+        'admin.group' => true,
+        'preferences.password' => true,
+      },
       member:    [
         'some_user_login',
       ],
@@ -3093,11 +3100,18 @@ wait untill text in selector disabppears
     end
 
     if data.key?(:permission)
-      data[:permission].each { |permission_name|
-        check(
-          browser: instance,
-          css:     ".modal [data-permission-name=\"#{permission_name}\"]",
-        )
+      data[:permission].each { |permission_name, permission_value|
+        if permission_value == false
+          uncheck(
+            browser: instance,
+            css:     ".modal [data-permission-name=\"#{permission_name}\"]",
+          )
+        else
+          check(
+            browser: instance,
+            css:     ".modal [data-permission-name=\"#{permission_name}\"]",
+          )
+        end
       }
     end