Permalink
Browse files

Fix for 1407 - Bad scroll performance since A4 on iPhone 3G

- Modified vmouse code so that it uses $.data() instead of $().data() which is significantly faster.

- Modified the navigation and buttonMarkup code so they stop using live(). The vmouse code triggers several events during the touch events, which in turn causes the underlying event code to call $.closest() with the selector used during the live() call to figure out if the event should be handled. This turns out to be very expensive, so instead, we now just bind directly to the document, and walk the DOM manually to figure out if we should handle it. This is much faster since we are avoid triggering multiple nested function calls.
  • Loading branch information...
1 parent cdd8b74 commit c31ee33a0f5eee981842abd02e4b1f24a2618473 @jblas jblas committed Apr 18, 2011
Showing with 87 additions and 45 deletions.
  1. +37 −9 js/jquery.mobile.buttonMarkup.js
  2. +32 −14 js/jquery.mobile.navigation.js
  3. +18 −22 js/jquery.mobile.vmouse.js
@@ -81,23 +81,51 @@ $.fn.buttonMarkup.defaults = {
wrapperEls: "span"
};
+function closestEnabledButton(element)
+{
+ while (element){
+ var $ele = $(element);
+ if ($ele.hasClass("ui-btn") && !ele.hasClass("ui-disabled")){
+ break;
+ }
+ element = element.parentNode;
+ }
+ return element;
+}
+
var attachEvents = function() {
- $(".ui-btn:not(.ui-disabled)").live({
+ $(document).bind({
"vmousedown": function() {
- var theme = $(this).attr( "data-" + $.mobile.ns + "theme" );
- $(this).removeClass( "ui-btn-up-" + theme ).addClass( "ui-btn-down-" + theme );
+ var btn = closestEnabledButton(this);
@staabm
staabm Apr 19, 2011 Contributor

why not do
$(this).parents(".ui-btn:not(.ui-disabled):first")

instead of using a separate function?

@jblas
jblas Apr 19, 2011 Contributor

The point of the optimization was to try and make as few underlying function calls as possible because these functions will get called for pretty much any touch/mouse event that gets triggered. Calling parents() with a filter selector would actually be worst than the the closest() function, which we are trying to avoid by not using the live() code, because it will be called for every ancestor of the element. The closest() function is what was making touch scroll "hiccup" because of the number of events triggered coupled with the number of times closest() was triggered to figure out if the live() handler should be fired.

+ if (btn){
+ var $btn = $(btn),
+ theme = $btn.attr( "data-" + $.mobile.ns + "theme" );
+ $btn.removeClass( "ui-btn-up-" + theme ).addClass( "ui-btn-down-" + theme );
+ }
},
"vmousecancel vmouseup": function() {
- var theme = $(this).attr( "data-" + $.mobile.ns + "theme" );
- $(this).removeClass( "ui-btn-down-" + theme ).addClass( "ui-btn-up-" + theme );
+ var btn = closestEnabledButton(this);
+ if (btn){
+ var $btn = $(btn),
+ theme = $btn.attr( "data-" + $.mobile.ns + "theme" );
+ $btn.removeClass( "ui-btn-down-" + theme ).addClass( "ui-btn-up-" + theme );
+ }
},
"vmouseover focus": function() {
- var theme = $(this).attr( "data-" + $.mobile.ns + "theme" );
- $(this).removeClass( "ui-btn-up-" + theme ).addClass( "ui-btn-hover-" + theme );
+ var btn = closestEnabledButton(this);
+ if (btn){
+ var $btn = $(btn),
+ theme = $btn.attr( "data-" + $.mobile.ns + "theme" );
+ $btn.removeClass( "ui-btn-up-" + theme ).addClass( "ui-btn-hover-" + theme );
+ }
},
"vmouseout blur": function() {
- var theme = $(this).attr( "data-" + $.mobile.ns + "theme" );
- $(this).removeClass( "ui-btn-hover-" + theme ).addClass( "ui-btn-up-" + theme );
+ var btn = closestEnabledButton(this);
+ if (btn){
+ var $btn = $(btn),
+ theme = $btn.attr( "data-" + $.mobile.ns + "theme" );
+ $btn.removeClass( "ui-btn-hover-" + theme ).addClass( "ui-btn-up-" + theme );
+ }
}
});
@@ -718,19 +718,37 @@
event.preventDefault();
});
+ function findClosestLink(ele)
+ {
+ while (ele){
+ if (ele.nodeName.toLowerCase() == "a"){
+ break;
+ }
+ ele = ele.parentNode;
+ }
+ return ele;
+ }
+
//add active state on vclick
- $( "a" ).live( "vclick", function(){
- $(this).closest( ".ui-btn" ).not( ".ui-disabled" ).addClass( $.mobile.activeBtnClass );
+ $( document).bind( "vclick", function(event){
+ var link = findClosestLink(event.target);
+ if (link){
+ $(link).closest( ".ui-btn" ).not( ".ui-disabled" ).addClass( $.mobile.activeBtnClass );
+ }
});
//click routing - direct to HTTP or Ajax, accordingly
- $( "a" ).live( "click", function(event) {
+ $( document ).bind( "click", function(event) {
+ var link = findClosestLink(event.target);
+ if (!link){
+ return;
+ }
- var $this = $(this),
+ var $link = $(link),
//get href, if defined, otherwise fall to null #
- href = $this.attr( "href" ) || "#",
+ href = $link.attr( "href" ) || "#",
//cache a check for whether the link had a protocol
//if this is true and the link was same domain, we won't want
@@ -742,7 +760,7 @@
url = path.clean( href ),
//rel set to external
- isRelExternal = $this.is( "[rel='external']" ),
+ isRelExternal = $link.is( "[rel='external']" ),
//rel set to external
isEmbeddedPage = path.isEmbeddedPage( url ),
@@ -761,13 +779,13 @@
isExternal = (path.isExternal(url) && !isCrossDomainPageLoad) || (isRelExternal && !isEmbeddedPage),
//if target attr is specified we mimic _blank... for now
- hasTarget = $this.is( "[target]" ),
+ hasTarget = $link.is( "[target]" ),
//if data-ajax attr is set to false, use the default behavior of a link
- hasAjaxDisabled = $this.is( ":jqmData(ajax='false')" );
+ hasAjaxDisabled = $link.is( ":jqmData(ajax='false')" );
//if there's a data-rel=back attr, go back in history
- if( $this.is( ":jqmData(rel='back')" ) ){
+ if( $link.is( ":jqmData(rel='back')" ) ){
window.history.back();
return false;
}
@@ -780,7 +798,7 @@
return;
}
- $activeClickedLink = $this.closest( ".ui-btn" );
+ $activeClickedLink = $link.closest( ".ui-btn" );
if( isExternal || hasAjaxDisabled || hasTarget || !$.mobile.ajaxEnabled ||
// TODO: deprecated - remove at 1.0
@@ -793,14 +811,14 @@
}
//use ajax
- var transition = $this.jqmData( "transition" ),
- direction = $this.jqmData("direction"),
+ var transition = $link.jqmData( "transition" ),
+ direction = $link.jqmData("direction"),
reverse = (direction && direction === "reverse") ||
// deprecated - remove by 1.0
- $this.jqmData( "back" );
+ $link.jqmData( "back" );
//this may need to be more specific as we use data-rel more
- nextPageRole = $this.attr( "data-" + $.mobile.ns + "rel" );
+ nextPageRole = $link.attr( "data-" + $.mobile.ns + "rel" );
//if it's a relative href, prefix href with base url
if( path.isRelative( url ) && !hadProtocol ){
@@ -91,28 +91,26 @@ function createVirtualEvent(event, eventType)
function getVirtualBindingFlags(element)
{
var flags = {};
- var $ele = $(element);
- while ($ele && $ele.length){
- var b = $ele.data(dataPropertyName);
+ while (element){
+ var b = $.data(element, dataPropertyName);
for (var k in b) {
if (b[k]){
flags[k] = flags.hasVirtualBinding = true;
}
}
- $ele = $ele.parent();
+ element = element.parentNode;
}
return flags;
}
function getClosestElementWithVirtualBinding(element, eventType)
{
- var $ele = $(element);
- while ($ele && $ele.length){
- var b = $ele.data(dataPropertyName);
+ while (element){
+ var b = $.data(element, dataPropertyName);
if (b && (!eventType || b[eventType])) {
- return $ele;
+ return element;
}
- $ele = $ele.parent();
+ element = element.parentNode;
}
return null;
}
@@ -177,7 +175,7 @@ function triggerVirtualEvent(eventType, event, flags)
function mouseEventCallback(event)
{
- var touchID = $(event.target).data(touchTargetPropertyName);
+ var touchID = $.data(event.target, touchTargetPropertyName);
if (!blockMouseTriggers && (!lastTouchID || lastTouchID !== touchID)){
triggerVirtualEvent("v" + event.type, event);
}
@@ -192,7 +190,7 @@ function handleTouchStart(event)
if (flags.hasVirtualBinding){
lastTouchID = nextTouchID++;
- $(target).data(touchTargetPropertyName, lastTouchID);
+ $.data(target, touchTargetPropertyName, lastTouchID);
clearResetTimer();
@@ -274,9 +272,9 @@ function handleTouchEnd(event)
startResetTimer();
}
-function hasVirtualBindings($ele)
+function hasVirtualBindings(ele)
{
- var bindings = $ele.data(dataPropertyName), k;
+ var bindings = $.data(ele, dataPropertyName), k;
if (bindings){
for (k in bindings){
if (bindings[k]){
@@ -297,16 +295,14 @@ function getSpecialEventObject(eventType)
// If this is the first virtual mouse binding for this element,
// add a bindings object to its data.
- var $this = $(this);
-
- if (!hasVirtualBindings($this)){
- $this.data(dataPropertyName, {});
+ if (!hasVirtualBindings(this)){
+ $.data(this, dataPropertyName, {});
}
// If setup is called, we know it is the first binding for this
// eventType, so initialize the count for the eventType to zero.
- var bindings = $this.data(dataPropertyName);
+ var bindings = $.data(this, dataPropertyName);
bindings[eventType] = true;
// If this is the first virtual mouse event for this type,
@@ -321,7 +317,7 @@ function getSpecialEventObject(eventType)
// for elements unless they actually have handlers registered on them.
// To get around this, we register dummy handlers on the elements.
- $this.bind(realType, dummyMouseHandler);
+ $(this).bind(realType, dummyMouseHandler);
// For now, if event capture is not supported, we rely on mouse handlers.
if (eventCaptureSupported){
@@ -373,7 +369,7 @@ function getSpecialEventObject(eventType)
}
var $this = $(this),
- bindings = $this.data(dataPropertyName);
+ bindings = $.data(this, dataPropertyName);
bindings[eventType] = false;
// Unregister the dummy event handler.
@@ -383,7 +379,7 @@ function getSpecialEventObject(eventType)
// If this is the last virtual mouse binding on the
// element, remove the binding data from the element.
- if (!hasVirtualBindings($this)){
+ if (!hasVirtualBindings(this)){
$this.removeData(dataPropertyName);
}
}
@@ -440,7 +436,7 @@ if (eventCaptureSupported){
for (var i = 0; i < cnt; i++) {
var o = clickBlockList[i],
touchID = 0;
- if ((ele === target && Math.abs(o.x - x) < threshold && Math.abs(o.y - y) < threshold) || $(ele).data(touchTargetPropertyName) === o.touchID){
+ if ((ele === target && Math.abs(o.x - x) < threshold && Math.abs(o.y - y) < threshold) || $.data(ele, touchTargetPropertyName) === o.touchID){
// XXX: We may want to consider removing matches from the block list
// instead of waiting for the reset timer to fire.
e.preventDefault();

0 comments on commit c31ee33

Please sign in to comment.