farmOS / farmOS-map

farmOS-map is an OpenLayers wrapper library designed for agricultural mapping needs. It can be used in any project that has similar requirements.
https://farmOS.github.io/farmOS-map
MIT License
39 stars 22 forks source link

Cannot click & drag the map in desktop Safari #88

Open paul121 opened 4 years ago

paul121 commented 4 years ago

@vital-agronomics noticed we can't click & drag navigate the map in Desktop safari. I was able to recreate on my Mac laptop.

We can click on the map controls (zoom in/zoom out, layer control, etc) but cannot click and drag the map around. We've tried mouse and trackpad. Also are aware that we need to "click" on the map before dragging. The map container is highlighted a light blue after clicking. Nothing is displayed in the console.

Unfortunately both our laptops are fairly old and not sure if we can even upgrade to the latest version of Safari/Mac OS. But here are versions we have tested.

I might have access to a newer Mac I could test this on.. but a client we are working with has older macbooks on the farm, so hopefully upgrading isn't the only solution. Haven't a chance to test much further, just wanted to create the issue now!

mstenta commented 4 years ago

Hmm that's frustrating.

I would be curious to see if the "click to focus" is causing this or not. It should be easy enough to test:

  1. Clone the repo on the Mac in question
  2. Run npm run dev and open http://localhost:8080 in Safari - confirm that the map works as expected (no issues).
  3. Edit static/index.html:
    • Within the options object, add interactions: { onFocusOnly: true}
    • Within the <div id="map"></div> tag, add the attribute: tabindex="0"
  4. Reload the dev site and test again (re-run npm run dev and reload browser). Does the issue occur?

@paul121 if you have a chance can you try that and see if it makes any difference?

mstenta commented 4 years ago

Found this: https://stackoverflow.com/questions/42758815/safari-focus-event-doesnt-work-on-button-element

It doesn't describe this issue exactly, but it does suggest that Safari's rules about what creates "focus" are different, which could be a clue. One of the solutions/suggestions in there is to listen for a click event on the element and trigger focus on it via JS. Not sure if that would help here...

Maybe clicking on the map is giving focus to SOME element (hence why you are seeing "highlighted a light blue after clicking"), but maybe it's not the RIGHT element... ?

paul121 commented 4 years ago

Found the issue!

OL v6.4.0 release notes:

Now that all major browsers support Pointer events natively, we removed the elm-pep dependency. If you are targeting older browsers that do not support Pointer events, you now need to include a pointer events polyfill (elm-pep or pepjs) in your application.

For reference, the polyfill was removed with https://github.com/openlayers/openlayers/pull/11173

Simple fix is re-adding the elm-pip dependency and including the polyfill in main.js.


@mstenta I tried all of your steps above and the map wasn't working at step 2. None of the custom edit controls were working either (edit & snapping controls). But the latest OL drag example was working in Safari... then I noticed all of their examples have this in their HTML:

<!-- Pointer events polyfill for old browsers, see https://caniuse.com/#feat=pointer -->
<script src="https://unpkg.com/elm-pep"></script>

Seems weird they would remove the dependency when it is largely needed, but looks like the removal may have been motivated by a licensing issue: https://github.com/openlayers/openlayers/issues/11138 It doesn't seem like this would affect farmOS, but perhaps it would affect anyone who bundles the farmOS-map library (not sure if that is the case right now anyways) but might be worth including in the docs?

Or maybe we should consider using pepjs (https://github.com/jquery/PEP) for the pointer events polyfill? I believe this is what OL was using prior to elm-pep (https://github.com/mpizenberg/elm-pep), but they switched because elm-pep is more minimal (made builds 17kB smaller). Here is the PR that made the change: https://github.com/openlayers/openlayers/pull/10318

paul121 commented 4 years ago

This link lays out the effected browsers pretty well: https://caniuse.com/#feat=pointer

The date relative view is handy. That shows Firefox & Chrome have been supported since 2018 and Safari halfway through 2019. Notably, the latest Firefox on Android seems like it still isn't supported.

mstenta commented 4 years ago

Thanks for digging into this @paul121 !

looks like the removal may have been motivated by a licensing issue: openlayers/openlayers#11138 It doesn't seem like this would affect farmOS, but perhaps it would affect anyone who bundles the farmOS-map library (not sure if that is the case right now anyways) but might be worth including in the docs?

I'd like to understand this a bit better before we proceed. I don't know enough about the MPL to understand what is/isn't allowed.

According to Wikipedia:

Licenses common to free and open-source software (FOSS) are not necessarily compatible with each other,[16] and this can make it legally impossible to mix (or link) open-source code if the components have different licenses. For example, software that combined code released under version 1.1 of the Mozilla Public License (MPL) with code under the GNU General Public License (GPL) could not be distributed without violating one of the terms of the licenses;[17][better source needed] this despite both licenses being approved by both the Open Source Initiative[18] and the Free Software Foundation.[19]

https://en.wikipedia.org/wiki/License_compatibility#Compatibility_of_FOSS_licenses

The other consideration we have is drupal.org packaging license requirements. See https://www.drupal.org/docs/develop/packaging-a-distribution/drupalorg-distribution-packaging-requirements

Worth noting, we are considering moving off of drupal.org for farmOS 2.x packaging, but 7.x-1.x will continue to be packaged on drupal.org, so we are bound by their license requirements.

On a related note, I just opened #90 to consider changing the farmOS-map license to MIT.

So I think the questions are:

  1. Can we include MPL licensed code in farmOS-map as it is licensed now (GPLv2)?
  2. Can we include MPL licensed code in farmOS-map if we change to MIT (#90)?
  3. If MPL is not compatible with drupal.org policy, but it is part of a project that IS compatible (eg: GPLv2 or MIT), does that make it compatible?

Obligatory "I am not a lawyer." :-)

mstenta commented 4 years ago

From that link (https://github.com/openlayers/openlayers/issues/11138):

our foss complience tool says that you started to use a library with an MPL-2 license (elm-pep), and that's not allowed for us in the company. Our foss compliance directives (and probably the most companies that are affected by the same controls we are...also soon or late everywhere in the EU), will have problems with software using this kind of libraries, because "it could be" problematic, the lawyers said. I know that there are workarounds that in several cases would help, but unfortunately not in ours :(

I'm also wary of introducing something that is already a problem for someone else. :-/

jgaehring commented 4 years ago

Seems like there are lots of other options out there for pointer event polyfills, no?

paul121 commented 4 years ago

All good things to consider. Another option may be to use the pepjs (https://github.com/jquery/PEP) polyfill instead - I believe it has a CC0 1.0 license (public domain?): http://creativecommons.org/publicdomain/zero/1.0/

Hopefully we could selectively import from that library to avoid a much larger file size?

paul121 commented 4 years ago

@jgaehring I'm only aware of these two because they were mentioned in the OL issues - but if there are other options we should definitely consider them!

mstenta commented 4 years ago

our foss complience tool says that you started to use a library with an MPL-2 license (elm-pep)

I'm also curious about this "foss compliance tool" now. I wonder if it would find anything else we are already doing.

(Just a curiosity - not blocker for this)