Batch Overlay Firefox Bugfix

In some cases in Firefox the batch action didn’t work correctly. Mouse events got triggered but not in the overlay. Instead they happened in the ticket list below.

Further research showed that the bug only appeared when the drag started in a table cell. When it started in a child element of the table cell, for example a link or a text node, the bug didn’t appear.

This sounds like this Firefox bug from 2005: https://bugzilla.mozilla.org/show_bug.cgi?id=301255. The solution described in the comments of the bug is to disable the user-selection in the table cell via css. I’t already quite hard to select something in the ticket overviews because a text selection starting and ending in the same row will open that ticket. Because of that I choose to go for this fix for now.

app/assets/javascripts/app/controllers/ticket_overview.coffee

@@ -22,8 +22,8 @@ class App.TicketOverview extends App.Controller
 
   events:
     'mousedown .item': 'startDragItem'
-    'mouseenter .js-hover-target': 'highlightBatchEntry'
-    'mouseleave .js-hover-target': 'unhighlightBatchEntry'
+    'mouseenter .js-batch-hover-target': 'highlightBatchEntry'
+    'mouseleave .js-batch-hover-target': 'unhighlightBatchEntry'
 
   constructor: ->
     super
@@ -56,7 +56,6 @@ class App.TicketOverview extends App.Controller
     # TODO: fire @cancelDrag on ESC
 
   dragItem: (event) =>
-    return if !@batchSupport
     pos = @batchDragger.data()
     threshold = 3
     x = event.pageX - pos.dx
@@ -64,10 +63,7 @@ class App.TicketOverview extends App.Controller
     dir = if event.pageY > pos.startY then 1 else -1
 
     if !pos.moved
-      # trigger when moved a little up or down
-      # but don't trigge when moved left or right
-      # because the user might just want to select text
-      if Math.abs(event.pageY - pos.startY) - Math.abs(event.pageX - pos.startX) > threshold
+      if Math.abs(event.pageY - pos.startY) > threshold || Math.abs(event.pageX - pos.startX) > threshold
         @batchDragger.data 'moved', true
         @el.addClass('u-no-userselect')
         # check grabbed items batch checkbox to make sure its checked
@@ -108,8 +104,6 @@ class App.TicketOverview extends App.Controller
     $.Velocity.hook @batchDragger, 'translateY', "#{y}px"
 
   endDragItem: (event) =>
-    return if !@batchSupport
-    @mainContent.removeClass('u-unclickable')
     $(document).off 'mousemove.item'
     $(document).off 'mouseup.item'
     pos = @batchDragger.data()
@@ -179,8 +173,6 @@ class App.TicketOverview extends App.Controller
         duration: 300
 
   performBatchAction: (items, action, id) ->
-    console.log "perform action #{action} with id #{id} on #{items.length} checked items"
-
     if action is 'macro'
       @batchCount = items.length
       @batchCountIndex = 0
@@ -237,8 +229,7 @@ class App.TicketOverview extends App.Controller
       return
 
   showBatchOverlay: ->
-    @mainContent.addClass('u-unclickable')
-    @batchOverlay.show()
+    @batchOverlay.addClass('is-visible')
     $('html').css('overflow', 'hidden')
     @batchOverlayBackdrop.velocity { opacity: [1, 0] }, { duration: 500 }
     @batchMacroOffset = @batchMacro.offset().top + @batchMacro.outerHeight()
@@ -247,12 +238,11 @@ class App.TicketOverview extends App.Controller
     $(document).on 'mousemove.batchoverlay', @controlBatchOverlay
 
   hideBatchOverlay: ->
-    @mainContent.removeClass('u-unclickable')
     $(document).off 'mousemove.batchoverlay'
     @batchOverlayShown = false
     @batchOverlayBackdrop.velocity { opacity: [0, 1] }, { duration: 300, queue: false }
     @hideBatchCircles =>
-      @batchOverlay.hide()
+      @batchOverlay.removeClass('is-visible')
 
     $('html').css('overflow', '')
 
@@ -1068,7 +1058,7 @@ class Table extends App.Controller
         if @lastChecked && e.shiftKey
           # check items in a row
           currentItem = $(e.currentTarget).parents('.item')
-          lastCheckedItem = @lastChecked.parents('.item')
+          lastCheckedItem = $(@lastChecked).parents('.item')
           items = currentItem.parent().children()
 
           if currentItem.index() > lastCheckedItem.index()
@@ -1082,7 +1072,7 @@ class Table extends App.Controller
 
           items.slice(startId+1, endId).find('[name="bulk"]').prop('checked', (-> !@checked))
 
-        @lastChecked = $(e.currentTarget)
+        @lastChecked = e.currentTarget
       callbackIconHeader = (headers) ->
         attribute =
           name:        'icon'

app/assets/javascripts/app/views/ticket_overview/batch_overlay_macro.jst.eco

@@ -1,9 +1,7 @@
 <div class="batch-overlay-box-inner">
 <% for macro in @macros: %>
-  <div class="batch-overlay-macro-entry js-batch-overlay-entry" data-action="macro" data-id="<%= macro.id %>">
-  	<div class="js-hover-target">
+  <div class="batch-overlay-macro-entry js-batch-overlay-entry js-batch-hover-target" data-action="macro" data-id="<%= macro.id %>">
     <div class="batch-overlay-macro-entry-name"><%= macro.name %></div>
   </div>
-  </div>
 <% end %>
 </div>

app/assets/javascripts/app/views/ticket_overview/batch_overlay_user_group.jst.eco

@@ -1,12 +1,12 @@
 <% for user in @users: %>
 <div class="batch-overlay-assign-entry js-batch-overlay-entry" data-action="user_assign" data-id="<%= user.id %>">
-  <div class="js-hover-target"><%- user.avatar(80) %></div>
+  <div class="js-batch-hover-target"><%- user.avatar(80) %></div>
   <div class="batch-overlay-assign-entry-name"><%- user.displayName() %></div>
 </div>
 <% end %>
 <% for group in @groups: %>
 <div class="batch-overlay-assign-entry js-batch-overlay-entry" data-action="group_assign" data-id="<%= group.id %>">
-  <span class="avatar avatar--organization size-80 js-hover-target">
+  <span class="avatar avatar--organization size-80 js-batch-hover-target">
     <%- @Icon('organization') %>
   </span>
   <div class="batch-overlay-assign-entry-name"><%- group.displayName() %></div>

app/assets/stylesheets/zammad.scss

@@ -2751,6 +2751,10 @@ footer {
       display: none;
     }
   }
+
+  .table > tbody > tr > td {
+    user-select: none;
+  }
 }
 
 .overview-header {
@@ -8395,7 +8399,7 @@ output {
 
 .batch-overlay {
   @extend .fit;
-  z-index: 1;
+  z-index: 100;
   color: white;
   text-transform: uppercase;
   text-align: center;
@@ -8408,6 +8412,10 @@ output {
   overflow: hidden;
   user-select: none;
   
+  &.is-visible {
+    display: block;
+  }
+  
   &-backdrop {
     @extend .fit;
     background: hsla(231,20%,8%,.8);