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

Fix regexes that break Leaflet inside a multi line comment block, fixes #1288 #1607

Merged
merged 2 commits into from
Apr 19, 2013

Conversation

kristerkari
Copy link

This is a fix for issue #1288.

Currently the */ characters inside Leaflet's regular expressions break multi line comment blocks that can be used for lazy evaluating javascript [source 1] [source 2].

In addition to working around the regular expressions, I have also added L.Util.trim which with these changes is used by L.Util.splitWords and L.DomUtil.removeClass. If adding trim method is a problem, I can provide an alternative workaround.

trim and removeClass methods are the same ones used in Pikaday.js:
https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L73

List of changes:

  1. Added L.Util.trim method.
  2. Changed L.Util.splitWords to call L.Util.trim.
  3. Replaced L.DomUtil.removeClass with a new one to avoid having a regular expression with */. Also uses L.Util.trim.
  4. Added parentheses around the regular expression in Path.VML.js to avoid having a regular expression with */.

Linting and tests:

Checking for JS errors...
    Check passed
................................................................................
................................................................................
....................................
PhantomJS 1.9 (Mac): Executed 196 of 196 SUCCESS (1.393 secs / 0.727 secs)

Let me know if any changes are needed.

@mourner
Copy link
Member

mourner commented Apr 19, 2013

Looks pretty good! Thanks!

mourner added a commit that referenced this pull request Apr 19, 2013
Fix regexes that break Leaflet inside a multi line comment block, fixes #1288
@mourner mourner merged commit dfed54a into Leaflet:master Apr 19, 2013
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 this pull request may close these issues.

2 participants