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

Cannot click & drag the map in desktop Safari #88

Open
paul121 opened this issue Sep 19, 2020 · 10 comments
Open

Cannot click & drag the map in desktop Safari #88

paul121 opened this issue Sep 19, 2020 · 10 comments
Labels
bug Something isn't working

Comments

@paul121
Copy link
Member

paul121 commented Sep 19, 2020

@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.

  • macOS 10.14.2 & Safari 12.0.2
  • macOS 10.13.6 & Safari 11.1.2

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 mstenta added the bug Something isn't working label Sep 19, 2020
@mstenta
Copy link
Member

mstenta commented Sep 19, 2020

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
Copy link
Member

mstenta commented Sep 19, 2020

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
Copy link
Member Author

paul121 commented Sep 19, 2020

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 openlayers/openlayers#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: 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?

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: openlayers/openlayers#10318

paul121 added a commit to paul121/farmOS-map that referenced this issue Sep 19, 2020
paul121 added a commit to paul121/farmOS-map that referenced this issue Sep 19, 2020
paul121 added a commit to paul121/farmOS-map that referenced this issue Sep 19, 2020
@paul121
Copy link
Member Author

paul121 commented Sep 19, 2020

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
Copy link
Member

mstenta commented Sep 21, 2020

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 (Should we convert license from GPLv2 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
Copy link
Member

mstenta commented Sep 21, 2020

From that link (openlayers/openlayers#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
Copy link
Member

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

@paul121
Copy link
Member Author

paul121 commented Sep 21, 2020

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
Copy link
Member Author

paul121 commented Sep 21, 2020

@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
Copy link
Member

mstenta commented Sep 21, 2020

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants