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

Cleanup injected attribution <style> and <script> tags #1018

Closed
jfolds opened this issue Oct 25, 2017 · 4 comments · Fixed by #1029
Closed

Cleanup injected attribution <style> and <script> tags #1018

jfolds opened this issue Oct 25, 2017 · 4 comments · Fixed by #1029

Comments

@jfolds
Copy link
Contributor

jfolds commented Oct 25, 2017

<style> and <script> tags are injected into the page for attribution reasons when adding basemap layers. There should probably be a way to either: a) prevent injecting <script> or <style> tags into the page or b) remove the injected tags when a map is destroyed
@jfolds
Copy link
Contributor Author

jfolds commented Nov 10, 2017

Would adding a map "unload" event listener here: https://github.com/Esri/esri-leaflet/blob/master/src/Util.js#L199 be a valid option? something like:

    map.on('unload', function() {
      hoverAttributionStyle.remove();
      attributionStyle.remove();
    });

The above not accounting for <script> tags inserted. Would adding classes to the scripts be reasonable?

    map.on('unload', function() {
      hoverAttributionStyle.remove();
      attributionStyle.remove();
      // remove scripts with custom class name
      var nodeList = document.getElementsByClassName('esri-leaflet-jsonp-script');
      for (var i = 0; i < nodeList.length; i++) {
        nodeList.item(i).remove();
      }
    });

Anyway, just some suggestions to get the conversation started.

Thanks!

@jgravois
Copy link
Contributor

all that seems reasonable enough to me. thanks for chewing on it!

@jfolds
Copy link
Contributor Author

jfolds commented Nov 22, 2017

So, this works for removing scripts/styles on unload:
dtsagile@2bd4f65

However, tests now fail...having a hard time deciphering the issue. Even adding an empty "unload" event in Util.js causes test failures for me. (Map container instance is being reused?)

Any advice welcome, thanks!

OS: Windows 10
Node version: 6.10.1
NPM version: 3.10.10

image

@jgravois
Copy link
Contributor

its possible that my changes to the test suite in #1025 will help.

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 a pull request may close this issue.

2 participants