Skip to content

Commit

Permalink
Patch for #770
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonathanolson
jonathanolson committed Jun 5, 2018
1 parent db7ea87 commit a152222
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 21 deletions.
15 changes: 13 additions & 2 deletions js/display/Display.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ define( function( require ) {
// supertype call to axon.Events (should just initialize a few properties here, notably _eventListeners and _staticEventListeners)
Events.call( this );

// Inline platform detection in maintenance release, since usually we won't need to hit phet-core
var ua = navigator.userAgent;
// Whether the browser is most likely Safari running on iOS
// See http://stackoverflow.com/questions/3007480/determine-if-user-navigated-from-mobile-safari
function isMobileSafari() {
return !!( ua.match( /(iPod|iPhone|iPad)/ ) && ua.match( /AppleWebKit/ ) );
}
var safari = isMobileSafari() || !!( ua.match( /Version\// ) && ua.match( /Safari\// ) && ua.match( /AppleWebKit/ ) );

options = _.extend( {
// initial display width
width: ( options && options.container && options.container.clientWidth ) || 640,
Expand Down Expand Up @@ -153,7 +162,8 @@ define( function( require ) {
// a context restored event. Sometimes context losses can occur without a restoration afterwards, but this can
// jump-start the process.
// See https://github.com/phetsims/scenery/issues/347.
aggressiveContextRecreation: true
aggressiveContextRecreation: true,
passiveEvents: safari ? false : null
}, options );

// TODO: don't store the options, it's an anti-pattern.
Expand Down Expand Up @@ -216,6 +226,7 @@ define( function( require ) {
this._listenToOnlyElement = options.listenToOnlyElement; // TODO: doc
this._batchDOMEvents = options.batchDOMEvents; // TODO: doc
this._assumeFullWindow = options.assumeFullWindow; // TODO: doc
this._passiveEvents = options.passiveEvents;

// @public (scenery-internal) {boolean}
this._aggressiveContextRecreation = options.aggressiveContextRecreation;
Expand Down Expand Up @@ -1082,7 +1093,7 @@ define( function( require ) {
assert && assert( !this._input, 'Events cannot be attached twice to a display (for now)' );

// TODO: refactor here
var input = new Input( this, !this._listenToOnlyElement, this._batchDOMEvents, this._assumeFullWindow );
var input = new Input( this, !this._listenToOnlyElement, this._batchDOMEvents, this._assumeFullWindow, this._passiveEvents );
this._input = input;

input.connectListeners();
Expand Down
52 changes: 37 additions & 15 deletions js/input/BrowserEvents.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ define( function( require ) {

var arrayRemove = require( 'PHET_CORE/arrayRemove' );
var BatchedDOMEvent = require( 'SCENERY/input/BatchedDOMEvent' );
var Features = require( 'SCENERY/util/Features' );
var scenery = require( 'SCENERY/scenery' );

// Sometimes we need to add a listener that does absolutely nothing
Expand All @@ -25,7 +26,7 @@ define( function( require ) {
* @param {boolean} attachToWindow - Whether events should be attached to the window. If false, they will be
* attached to the Display's domElement.
*/
addDisplay: function( display, attachToWindow ) {
addDisplay: function( display, attachToWindow, passiveEvents ) {
assert && assert( display instanceof scenery.Display );
assert && assert( typeof attachToWindow === 'boolean' );
assert && assert( !_.includes( this.attachedDisplays, display ),
Expand All @@ -36,15 +37,15 @@ define( function( require ) {
if ( attachToWindow ) {
// lazily connect listeners
if ( this.attachedDisplays.length === 1 ) {
this.connectWindowListeners();
this.connectWindowListeners( passiveEvents );
}
}
else {
this.addOrRemoveListeners( display.domElement, true );
this.addOrRemoveListeners( display.domElement, true, passiveEvents );
}

// Only add the wheel listeners directly on the elements, so it won't trigger outside
display.domElement.addEventListener( 'wheel', this.onwheel, false );
display.domElement.addEventListener( 'wheel', this.onwheel, BrowserEvents.getEventOptions( passiveEvents, true ) );
},

/**
Expand All @@ -54,7 +55,7 @@ define( function( require ) {
* @param {Display} display
* @param {boolean} attachToWindow - The value provided to addDisplay
*/
removeDisplay: function( display, attachToWindow ) {
removeDisplay: function( display, attachToWindow, passiveEvents ) {
assert && assert( display instanceof scenery.Display );
assert && assert( typeof attachToWindow === 'boolean' );
assert && assert( _.includes( this.attachedDisplays, display ),
Expand All @@ -65,14 +66,32 @@ define( function( require ) {
// lazily disconnect listeners
if ( attachToWindow ) {
if ( this.attachedDisplays.length === 0 ) {
this.disconnectWindowListeners();
this.disconnectWindowListeners( passiveEvents );
}
}
else {
this.addOrRemoveListeners( display.domElement, false );
this.addOrRemoveListeners( display.domElement, false, passiveEvents );
}

display.domElement.removeEventListener( 'wheel', this.onwheel, false );
display.domElement.removeEventListener( 'wheel', this.onwheel, BrowserEvents.getEventOptions( passiveEvents, true ) );
},

getEventOptions: function( passiveEvents, isMain ) {
var passDirectPassiveFlag = Features.passive && passiveEvents !== null;
if ( !passDirectPassiveFlag ) {
return false;
}
if ( isMain ) {
return {
useCapture: false,
passive: passiveEvents
};
}
else {
return {
passive: passiveEvents
};
}
},

/**
Expand Down Expand Up @@ -200,16 +219,16 @@ define( function( require ) {
* Connects event listeners directly to the window.
* @private
*/
connectWindowListeners: function() {
this.addOrRemoveListeners( window, true );
connectWindowListeners: function( passiveEvents ) {
this.addOrRemoveListeners( window, true, passiveEvents );
},

/**
* Disconnects event listeners from the window.
* @private
*/
disconnectWindowListeners: function() {
this.addOrRemoveListeners( window, false );
disconnectWindowListeners: function( passiveEvents ) {
this.addOrRemoveListeners( window, false, passiveEvents );
},

/**
Expand All @@ -219,7 +238,10 @@ define( function( require ) {
* @param {*} element - The element (window or DOM element) to add listeners to.
* @param {boolean} addOrRemove - If true, listeners will be added. If false, listeners will be removed.
*/
addOrRemoveListeners: function( element, addOrRemove ) {
addOrRemoveListeners: function( element, addOrRemove, passiveEvents ) {
var documentOptions = BrowserEvents.getEventOptions( passiveEvents, false );
var mainOptions = BrowserEvents.getEventOptions( passiveEvents, true );

assert && assert( typeof addOrRemove === 'boolean' );

var forWindow = element === window;
Expand Down Expand Up @@ -247,13 +269,13 @@ define( function( require ) {
// If we add input listeners to the window itself, iOS Safari 7 won't send touch events to displays in an
// iframe unless we also add dummy listeners to the document.
if ( forWindow ) {
document[ method ]( type, noop );
document[ method ]( type, noop, documentOptions );
}

var callback = this[ 'on' + type ];
assert && assert( !!callback );

element[ method ]( type, callback, false ); // false: don't use event capture for now
element[ method ]( type, callback, mainOptions ); // false: don't use event capture for now
}
},

Expand Down
9 changes: 5 additions & 4 deletions js/input/Input.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ define( function( require ) {
pointerType: true, charCode: true, which: true, clientX: true, clientY: true, changedTouches: true
};

function Input( display, attachToWindow, batchDOMEvents, assumeFullWindow ) {
function Input( display, attachToWindow, batchDOMEvents, assumeFullWindow, passiveEvents ) {
assert && assert( display instanceof scenery.Display );
assert && assert( typeof attachToWindow === 'boolean' );
assert && assert( typeof batchDOMEvents === 'boolean' );
Expand All @@ -153,6 +153,7 @@ define( function( require ) {
this.attachToWindow = attachToWindow;
this.batchDOMEvents = batchDOMEvents;
this.assumeFullWindow = assumeFullWindow;
this.passiveEvents = passiveEvents;
this.displayUpdateOnEvent = false;

this.batchedEvents = [];
Expand Down Expand Up @@ -188,7 +189,7 @@ define( function( require ) {
// http://www.html5rocks.com/en/mobile/touchandmouse/ for more information.
// Additionally, IE had some issues with skipping prevent default, see
// https://github.com/phetsims/scenery/issues/464 for mouse handling.
if ( callback !== this.mouseDown || platform.ie || platform.edge ) {
if ( !( this.passiveEvents === true ) && ( callback !== this.mouseDown || platform.ie || platform.edge ) ) {
domEvent.preventDefault();
}
},
Expand Down Expand Up @@ -235,11 +236,11 @@ define( function( require ) {
},

connectListeners: function() {
BrowserEvents.addDisplay( this.display, this.attachToWindow );
BrowserEvents.addDisplay( this.display, this.attachToWindow, this.passiveEvents );
},

disconnectListeners: function() {
BrowserEvents.removeDisplay( this.display, this.attachToWindow );
BrowserEvents.removeDisplay( this.display, this.attachToWindow, this.passiveEvents );
},

pointFromEvent: function( domEvent ) {
Expand Down
9 changes: 9 additions & 0 deletions js/util/Features.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,14 @@ define( function( require ) {
}
};

// Whether passive is a supported option for adding event listeners,
// see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Improving_scrolling_performance_with_passive_listeners
Features.passive = false;
window.addEventListener( 'test', null, Object.defineProperty( {}, 'passive', {
get: function() {
Features.passive = true;
}
} ) );

return Features;
} );

0 comments on commit a152222

Please sign in to comment.