is it possible to leave getMousePosition less complex? E.g. move scaled point calculation to a separate util function, which will be used by getMousePosition and inside Draggable?

i think that shall make it more complex. Any calling of function has some overhead. Ant that's mouse event. Any single function call shall make slower response to user.

Now getMousePosition looks as only method to get right coordinates of mouse event. It's used now everywhere at Leaflet. I think that's right behavior.

Third argument i've added is not simple for understanding and even for description... but it's needed in that situation I set it to true. We can try to avoid it... but i think it's better for now to add it. Ternary operator is very atomic and fast.

Third argument i've added is not simple for understanding and even for description

That's what I meant under "complexity".

Right now getMousePosition is easy to reason about. With abs argument it's much harder to do it.
Same with mapContainer option in Draggable. Right now Draggable doesn't care about map container at all - and in most cases it shouldn't.

Personally, I would rather not add this complexity to Leaflet core, unless there's simpler / more readable solution for that.
It's a corner case and 99% of users do not need this. So I'd stick with current, less complex solution.

From the other hand, I understand that you need this to work and I really appreciate your efforts!
But maybe it's possible to pull it out into a plugin (maybe with some additional work on Leaflet side to make it possible)? If I can help you with that - just let me know :)

That being said, it's just my personal opinion, and maybe other core contributors have other thoughts.
cc @mourner @perliedman @IvanSanchez @ghybs @yohanboniface

@cherniavskii I agree this makes a core function a bit more complex, but pulling this fix out of the core to a plugin sounds a bit weird to me.

Is passing mapContainer to all Draggables that bad? If the container's attributes are needed to calculate the position of Draggable, I don't see any other way than passing the info down there. One option would be to only pass the offset x and y in numbers, but those would need to be updated when the container changes.

Would it be possible to name and document the abs better?

Hi,

It seems to me here that the abs parameter is unnecessary: the only 2 places where this PR uses it are in Draggable's _onDown and _onMove listeners. The results are then subtracted to compute the move offset (extent).

So we have: (pseudo-code)

offset = movePointAbs - startPointAbs
       = getMousePosition(movePoint, mapContainer, true) - getMousePosition(startPoint, mapContainer, true)
       = Point(move_localX + move_rect.left, move_localY + move_rect.top) - Point(start_localX + start_rect.left, start_localY + rect.top)
       = Point(move_localX - start_localX, move_localY - start_localY) + Point(move_rect.left - start_rect.left,  move_rect.top - start_rect.top)

Since it is an extremely corner case that mapContainer changes position during the drag operation, then move_rect === start_rect and we have:

Point(move_rect.left - start_rect.left,  move_rect.top - start_rect.top) === 0
offset = Point(move_localX - start_localX, move_localY - start_localY)
       = Point(move_localX, move_localY) - Point(start_localX, start_localY)
       = getMousePosition(movePoint, mapContainer) - getMousePosition(startPoint, mapContainer)
       = movePoint - startPoint

Furthermore, the description for getMousePosition sounds pretty clear to me that it is relative to the container:

Gets normalized mouse position from a DOM event relative to the container

The only case where including the start and in-movement offsets in the computation would be interesting is when we expect the map container to be shifted while dragging. In that case, the drag could indeed continue smoothly, even though the mouse / finger no longer holds the same spot… But I think we could expect the user to already feel a discrepancy (when the map container moves), and wonder why he/she is no longer holding the same spot.

Without including this abs offset, on the contrary the map "jumps" back to where the mouse / finger was holding, which IMHO makes the user experience less disruptive.
Demo: https://plnkr.co/edit/poTcoZRUYKBRxvWnHoZp?p=preview

does it mean that mapContainer is required for every single usage of Draggable?

@cherniavskii not required, but optional. if you haven't specified it nothing shall be broken. But i must give container instance to getMousePosition, it must have access to it. haven't other idea how to have that access in better way.

As @yarsky-tgz figured out, in order to have Draggable correctly compute the drag offset, taking into account the map container CSS scale, it must have a reference somehow to that map container. This is easy for Map.Drag, but not for Marker.Drag.

With the current PR state, this new mapContainer option is indeed optional.
Draggable behaves as previously when this option is not set (i.e. it uses the pointer position directly on screen).

Thinking about it further, this shows that Draggable parameters are simply not enough:

1. Moving element
2. Drag target
3. (prevent outline)
4. (options)

…but what about the drag area, which is potentially affected by a CSS scale?
Again, in the case of Map.Drag, we simply have Drag target === drag area, but that is not the case for Marker.Drag (the icon can have its own CSS scale).

Therefore adding a new parameter or option is necessary in order to be able to take into account the container CSS scale. Maybe a more generic dragArea or dragContainer name could be used?

…or maybe "dragScaledContainer", to make it more obvious that this option is used to retrieve the CSS scale and correct the computed movement accordingly, rather than the area on which the drag is effective (drag does not stop if user points outside the dragTarget, or whatever container).

No reference to "map" is needed, since Draggable can be applied to any arbitrary Element, not necessarily something related to a Leaflet map:
https://plnkr.co/edit/m2LUcgmcbOSvliuANumg?p=preview

All right, there should be an interesting workaround that would enable getting rid of the mapContainer option.

What we need is to retrieve the CSS scale which applies on the dragged element (i.e. this._element in Draggable instance), except for a potential scale on the very element itself. This scale is determined by the combined scales of the hierarchy of DOM parent nodes of the dragged element. Therefore reading the scale of the element's parentNode should be enough to compute the scale correction.

This works fine for the map, but for a draggable Marker, the difficulty is that the leaflet-marker-pane is of size 0, which makes the scale computation algorithm (as implemented here) fail. So we can walk up the DOM parent nodes hierarchy until we find an element of size !== 0, crossing fingers that intermediate elements maintain the CSS scale. That is the case for the leaflet-map-pane and the leaflet-marker-pane.

function getNonNullSizeParentNode(element) {
  do {
    element = element.parentNode;
  } while ((!element.offsetWidth || !element.offsetHeight) && element !== document.body)
  
  return element;
}

Example: https://plnkr.co/edit/HYyDssl2F2rpvwmDYuW6?p=preview