Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't register mouse events if pointer events are supported #1614

Merged

Conversation

rachel-fenichel
Copy link
Collaborator

@rachel-fenichel rachel-fenichel commented Feb 14, 2018

core/blockly.js Outdated
var bindData = [[node, name, wrapFunc]];
var bindData = [];
// Don't register the mouse event if an equivalent pointer event is supported.
if (!window.PointerEvent || !(name in Blockly.Touch.TOUCH_MAP)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window might not exist in headless mode. Are we certain this function will never be called in headless? I feel more comfortable checking window too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. That also means that there should be a check for window in touch.js:

Blockly.Touch.TOUCH_MAP = {};
if (window.PointerEvent) {
  Blockly.Touch.TOUCH_MAP = {
    'mousedown': ['pointerdown'],
    'mousemove': ['pointermove'],
    'mouseup': ['pointerup', 'pointercancel']
  };
} else if (goog.events.BrowserFeature.TOUCH_ENABLED) {
  Blockly.Touch.TOUCH_MAP = {
    'mousedown': ['touchstart'],
    'mousemove': ['touchmove'],
    'mouseup': ['touchend', 'touchcancel']
  };
}

In the particular case of bindEvent_, it's called from inject.js with window as the node argument, so we may have already failed.

It looks like we used to create a fake window object to solve this problem (not the best solution): https://github.com/google/blockly/pull/1566/files Now that's gone.

core/blockly.js Outdated
// Add equivalent touch event.
var bindData = [];
// Don't register the mouse event if an equivalent pointer event is supported.
if (!window.PointerEvent || !(name in Blockly.Touch.TOUCH_MAP)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern with window

@rachel-fenichel
Copy link
Collaborator Author

This still doesn't work--now I get zero clicks in firefox and one click in chrome, instead of one click in firefox and two in chrome. I'm disabling them for this release.

@rachel-fenichel rachel-fenichel merged commit 7356c66 into google:develop Mar 2, 2018
@rachel-fenichel
Copy link
Collaborator Author

Merged, because @samelhusseini proved that the problem was with the firefox simulator.

@rachel-fenichel rachel-fenichel deleted the bugfix/double_events branch March 2, 2018 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants