Stop event propagation for {brush,drag,zoom}.

[jasondavies]

This allows behaviours and d3.svg.brush to be used together easily.

Also, only bind relevant event listeners on brush touchstart; a
mousemove event was being triggered on mobile Safari causing the brush
to jump briefly on touchstart, e.g. when viewing:

http://bl.ocks.org/mbostock/raw/1667367/

Fixes #1321.

[jasondavies]

Also suppresses user-select for brush.

[mbostock]

Ugh, this logic is becoming frustratingly complex. Why is it okay to preventDefault on mousemove and mouseup, but not on mousedown? Would it be simpler to revert all this extra logic for suppressing selectstart events and just cancel the mousedown event, but focus the window? Do we still want a d3_eventCancel method if most of the places we use it, we just mean to stopPropagation? Should d3_eventCancel be replaced with two methods, d3_eventStopPropagation and d3_eventPreventDefault to shave a few bytes?

[jasondavies]

Yeah, I’m also concerned that having no preventDefault on mousedown makes us brittle to arbitrary browser defaults that may be added in future (or exist already, like the image dragstart issue). For example, touch-and-hold seems to be blocked by the user-select approach, but I could imagine touch-enabled devices coming up with other default behaviour that we haven’t thought of.

For @jfirebaugh’s specific issue, I think window.focus() would be fine. Even if we didn’t have window.focus(), he could add a mousedown listener of his own quite easily.

It is a shame that preventDefault makes dragging problematic within iframes, because it seems to block mousemove when the mouse moves outside of the iframe. This is one of the things that persuaded me to go for this approach in the first place. I tried to find an alternative that works with preventDefault, but no luck so far.

If there was a way to revert to the preventDefault approach, but fix dragging in iframes, that would be perfect. Otherwise, I’m a bit torn, but I think reverting is probably safer on balance.

[jfirebaugh]

There was a window.focus() there prior to removing preventDefault. It didn't work. According to MDN window.focus() is for (maybe) bringing a window to the foreground, not for changing document.activeElement.

[jasondavies]

document.activeElement.blur() seems to work. It seems a bit invasive for the drag behaviour to do this though.

[mbostock]

I’ve written code in the past to find the first "focusable parent" of a clicked on element. The polymaps drag mousedown handler:

function mousedown(e) {
  if (e.shiftKey) return;
  dragging = {
    x: e.clientX,
    y: e.clientY
  };
  map.focusableParent().focus();
  e.preventDefault();
  document.body.style.setProperty("cursor", "move", null);
}

Where:

map.focusableParent = function() {
  for (var p = container; p; p = p.parentNode) {
    if (p.tabIndex >= 0) return p;
  }
  return window;
};

Not sure if that really helps in this case, though.

[mbostock]

Do you have any opinion on the desired behavior, @jfirebaugh? FWIW, Leaflet also seems to preventDefault and stopPropagation on mousedown (see Draggable.js), and so by extension I assume that’s what the MapBox JavaScript API does as a Leaflet plugin. As much as it’s nice to have mouseup work outside an iframe, I think given the other issues this is raising, I’m inclined to restore the previous behavior.

[mbostock]

Actually, it looks like Leaflet solves this problem by adding a tabindex attribute to the container div, along with an outline:0 style, so that even though Leaflet uses preventDefault and stopPropagation, it focuses this container so that clicking on a draggable element causes the focus to move. (It doesn’t work outside an iframe, though.)

Of course D3’s behaviors are typically bound to G elements within an SVG, so the behavior would probably need to grab the owner SVG element to apply this technique.

[jfirebaugh]

What exactly are the issues with never using preventDefault? @jasondavies raised the issue of future-proofedness, but it seems to me that could just as easily go the other way, with future default behavior being added that we would want to be triggered.

I haven't encountered any issues with stopPropagation, and in general that is easier to customize externally to the behavior by capturing or using bubble or callback binding order, so liberal use of stopPropagation seems less problematic.

[jfirebaugh]

It looks like Leaflet has a workaround for the stuck iframe pan issue as well.

[jasondavies]

Unfortunately, that’s not a fix for the same issue. See Leaflet/Leaflet#1277.

[mbostock]

If we don’t preventDefault on mousedown, then clicking and dragging will allow text selection, image dragging and link dragging as default behaviors, which can interfere with D3’s behavior. Jason’s previous implementation explicitly suppressed the selectstart event so as to prevent text selection, but this didn’t prevent image dragging (hence the need to add pointer-events: none to the d3.geo.tile example). The point is that if we don’t prevent the browser’s default behavior on mousedown, then unknown things will happen that may conflict with D3’s behavior. We can explicitly disable the ones we know about, but that seems more complex than just preventing the default behavior.

[mbostock]

It’s easy to verify that Leaflet doesn’t currently pan well inside an iframe—see this example.

[jfirebaugh]

I still come down on the side of not preventing default.

• Not preventing default requires that we find some way to prevent the specific default behaviors of text selection, image dragging and link dragging. It seems like we have a pretty good handle on this via canceling selectstart and dragstart.
• Preventing default causes focus problems, which perhaps can be worked around, and problems with dragging in iframes, which as far as we know cannot.
• We don't know the complete set of default behaviors. There may be existing default behaviors we haven't yet stumbled on, or additional default behaviors added in the future. Like selecting and dragging, some may be desirable to prevent. Conversely, like iframes and focus, some may be desirable to preserve. Overall, it's a wash.
• In general, it's easier to work around specific problems caused by not preventing default, either in d3 or in client code, than to work around problems caused by preventing default. If preventing default is problematic for a specific use case, the only recourse is to fork a copy of the behavior (which is what I've done in iD). But if preventing default is necessary for a specific use case, it's trivial to add externally.
• The relative complexity of canceling selectstart and dragstart vs. finding a focusable parent element seems like a wash to me.

[mbostock]

In general, it's easier to work around specific problems caused by not preventing default, either in d3 or in client code, than to work around problems caused by preventing default.

Do you think there are cases where a user would want different behavior than what D3 provides, and if so, can you give an example? In your case of iD, it seems to me you were fixing a bug in D3, not that you wanted different behavior. In other words, D3 tried to provide the “correct” behavior, but had a bug.

I’m happy for people to fork D3 to fix bugs, but I agree if users will want to override this behavior, then allowing default behaviors will make it easier. Or if you think it’s infeasible for D3 to ever implement this behavior correctly for all applications, that would be another argument to err on shifting the burden to users.

[jfirebaugh]

Good question. I don't have any specific examples of differing user needs with respect to any of the default behaviors we're aware of, so I suppose that part of the argument is more about the the potential for unknown or future default behaviors. If we leave preventing default up to the user, and it turns out there's a default behavior that's important to suppress, it's as feasible for the user to do so as it is for D3 -- feasibility here being dependent on relevant DOM APIs. But if D3 prevents default, and there's a default behavior that's important to preserve, than forking is necessary, at least until such time as D3 stops preventing default or makes it configurable.

The latter is the situation that iD is in with respect to zoom behavior in an iframe. On openstreetmap.org, iD is presented in an embedded iframe, so it's fairly important that panning behaves nicely when the cursor leaves the iframe bounds. If D3 goes back to preventing default, but we don't find a workaround for iframe drag behavior, I'll continue to bear the (modest) cost of maintaining a patched version in order to preserve the desired behavior.

[mbostock]

It is nice that this approach works in an iframe, so maybe that’s enough of an argument to continue with this approach even though it still feels (to me at least) more complex than preventing default behaviors.

Perhaps what’s missing here is finding a good way to unify the behaviors’ implementations. After all, brushing, dragging and zooming all involve the same basic mode of interaction: mousedown, mousemove*, mouseup (or the touch equivalents). Could we abstract this into a higher level gesture handler, and thereby consolidate the logic that deals with assigning the appropriate listeners and browser quirks?

[mbostock]

After discussion with Jason, closing this pull request. Will update #1321 with a description of the resolved strategy.