Skip to content

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

Merged
mourner merged 2 commits intoLeaflet:masterfrom
kristerkari:fix-js-lazyeval
Apr 19, 2013
Merged

Fix regexes that break Leaflet inside a multi line comment block, fixes #1288#1607
mourner merged 2 commits intoLeaflet:masterfrom
kristerkari:fix-js-lazyeval

Conversation

@kristerkari
Copy link
Copy Markdown

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
Copy Markdown
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