fix(click): fix mouseup click for ion-option-button

To allow select elements to change options in Firefox, the simulated tap click should not go through when the target is an `option` element. However, the regex was too general and also prevented `ion-option-button` on mouseup.
ionic-team · May 15, 2014 · 29ee640 · 29ee640
1 parent 462ef38
commit 29ee640
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 8 deletions.
diff --git a/js/utils/tap.js b/js/utils/tap.js
@@ -151,16 +151,16 @@ ionic.tap = {
   ignoreScrollStart: function(e) {
     return (e.defaultPrevented) ||  // defaultPrevented has been assigned by another component handling the event
            (e.target.isContentEditable) ||
-           (/file|range/i).test(e.target.type) ||
+           (/^(file|range)$/i).test(e.target.type) ||
            (e.target.dataset ? e.target.dataset.preventScroll : e.target.getAttribute('data-prevent-default')) == 'true' || // manually set within an elements attributes
-           (!!(/object|embed/i).test(e.target.tagName));  // flash/movie/object touches should not try to scroll
+           (!!(/^(object|embed)$/i).test(e.target.tagName));  // flash/movie/object touches should not try to scroll
   },
 
   isTextInput: function(ele) {
     return !!ele &&
            (ele.tagName == 'TEXTAREA' ||
             ele.contentEditable === 'true' ||
-            (ele.tagName == 'INPUT' && !(/radio|checkbox|range|file|submit|reset/i).test(ele.type)) );
+            (ele.tagName == 'INPUT' && !(/^(radio|checkbox|range|file|submit|reset)$/i).test(ele.type)) );
   },
 
   isLabelWithTextInput: function(ele) {
@@ -219,7 +219,7 @@ ionic.tap = {
   },
 
   requiresNativeClick: function(ele) {
-    if(!ele || ele.disabled || (/file|range/i).test(ele.type) || (/object|video/i).test(ele.tagName) ) {
+    if(!ele || ele.disabled || (/^(file|range)$/i).test(ele.type) || (/^(object|video)$/i).test(ele.tagName) ) {
       return true;
     }
     if(ele.nodeType === 1) {
@@ -325,7 +325,7 @@ function tapMouseUp(e) {
     return false;
   }
 
-  if( tapIgnoreEvent(e) || (/select|option/i).test(e.target.tagName) ) return;
+  if( tapIgnoreEvent(e) || (/^(select|option)$/i).test(e.target.tagName) ) return false;
 
   if( !tapHasPointerMoved(e) ) {
     tapClick(e);
@@ -378,7 +378,7 @@ function tapTouchEnd(e) {
   if( !tapHasPointerMoved(e) ) {
     tapClick(e);
 
-    if( (/select|option/i).test(e.target.tagName) ) {
+    if( (/^(select|option)$/i).test(e.target.tagName) ) {
       e.preventDefault();
     }
   }
@@ -435,7 +435,7 @@ function tapHandleFocus(ele) {
     // already is the active element and has focus
     triggerFocusIn = true;
 
-  } else if( (/input|textarea/i).test(ele.tagName) ) {
+  } else if( (/^(input|textarea)$/i).test(ele.tagName) ) {
     triggerFocusIn = true;
     ele.focus && ele.focus();
     ele.value = ele.value;
@@ -457,7 +457,7 @@ function tapHandleFocus(ele) {
 
 function tapFocusOutActive() {
   var ele = tapActiveElement();
-  if(ele && (/input|textarea|select/i).test(ele.tagName) ) {
+  if(ele && (/^(input|textarea|select)$/i).test(ele.tagName) ) {
     console.log('tapFocusOutActive', ele.tagName);
     ele.blur();
   }

diff --git a/test/unit/utils/tap.unit.js b/test/unit/utils/tap.unit.js
@@ -319,6 +319,30 @@ describe('Ionic Tap', function() {
     expect( e.preventedDefault ).toBeUndefined();
   });
 
+  it('Should not stop event on mouseup if the target was an option element', function() {
+    tapEnabledTouchEvents = false;
+    e = {
+      target: document.createElement('option'),
+      stopPropagation: function() { this.stoppedPropagation = true; },
+      preventDefault: function() { this.preventedDefault = true; }
+    };
+    expect( tapMouseUp(e) ).toEqual(false);
+    expect( e.stoppedPropagation ).toBeUndefined();
+    expect( e.preventedDefault ).toBeUndefined();
+  });
+
+  it('Should stop event on mouseup if the target was an ion-option-button element', function() {
+    tapEnabledTouchEvents = false;
+    e = {
+      target: document.createElement('ion-option-button'),
+      stopPropagation: function() { this.stoppedPropagation = true; },
+      preventDefault: function() { this.preventedDefault = true; }
+    };
+    expect( tapMouseUp(e) ).toBeUndefined();
+    expect( e.stoppedPropagation ).toBeUndefined();
+    expect( e.preventedDefault ).toBeUndefined();
+  });
+
   it('Should not stop event on mouseup if touch is not enabled', function() {
     tapEnabledTouchEvents = false;
     e = {