Code

Opened 5 years ago

Closed 2 years ago

#10625 closed Bug (fixed)

Ewkt regexp incorrectly escaped in GeoDjango admin javascript

Reported by: timlinux Owned by: springmeyer
Component: GIS Version: master
Severity: Normal Keywords: regexp ewkt
Cc: chris.chamberlin@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi

in http://code.djangoproject.com/browser/django/trunk/django/contrib/gis/templates/gis/admin/openlayers.js the regexp on line 3 is incorrectly escaped, causing conversion from ewkt to wkt to fail for me. I fixed it by changing:

new RegExp("^SRID=\d+;(.+)", "i");

to

new RegExp("^SRID=\\d+;(.+)", "i");

After which it correctly works for me. Would be great if you can apply this fix.

Thanks

Tim

Attachments (1)

10625.patch (1.0 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by Alex

  • milestone changed from 1.1 beta to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by jbronn

What browser version, please?

comment:3 Changed 5 years ago by psmith

I wasn't able to reproduce this. I tried:

  • Linux:
    • FF 3.0.9
    • Opera 9.6
  • Win XP:
    • FF 3.0.10
    • Opera 9.64
    • IE 6.0.x

Suggest this be punted to 1.2 since we don't have a user-agent yet of the reported bug.

comment:4 Changed 5 years ago by Alex

  • Resolution set to worksforme
  • Status changed from new to closed

Paul tried half a dozen browsers and we have no new information from the reported, marking as worksforme.

comment:5 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

comment:6 Changed 2 years ago by chris.chamberlin@…

  • Cc chris.chamberlin@… added
  • Easy pickings unset
  • Resolution worksforme deleted
  • Severity set to Normal
  • Status changed from closed to reopened
  • Summary changed from Ewkt regexp incorrectly escaped to Ewkt regexp incorrectly escaped in GeoDjango admin javascript
  • Type set to Bug
  • UI/UX set

I'm seeing this bug in Chrome 17.0.963.56; it's fairly simple to verify that the regexp does not match as designed, using a Javascript console:

> var brokenre = new RegExp("^SRID=\d+;(.+)", "i");
undefined
> brokenre.exec('SRID=4326;POINT(1 1)')
null
> var fixedre = new RegExp("^SRID=\\d+;(.+)", "i");
undefined
> fixedre.exec('SRID=4326;POINT(1 1)')
["SRID=4326;POINT(1 1)", "POINT(1 1)"]

It does not usually appear because the read_wkt() function is generally only called on startup, when the WKT textarea does not contain the EWKT format (with the SRID=4326; string that we're trying to strip).

It does appear, however, if read_wkt() is called again later; I wanted to be able to edit geometries by using either the map or the WKT textbox, so I added a JQuery change handler in an template as follows, mostly using code adapted from the openlayers.js file. In my GeoModelAdmin, I set map_template = "/path/to/my/template" and display_wkt=True.

{% extends "gis/admin/openlayers.html" %}

{% block init_function %}
{{ block.super }}
(function($) {
    $('.vWKTField').height('3em').change(function(e) {
        var wkt = e.target.value
        if (wkt){
            // After reading into geometry, immediately write back to
            // WKT <textarea> as EWKT (so that SRID is included).
            var admin_geom = {{ module }}.read_wkt(wkt);
            {{ module }}.deleteFeatures();
            {{ module }}.write_wkt(admin_geom);
            if ({{ module }}.is_collection){
                // If geometry collection, add each component individually so they may be
                // edited individually.
                for (var i = 0; i < {{ module }}.num_geom; i++){
                {{ module }}.layers.vector.addFeatures([new OpenLayers.Feature.Vector(admin_geom.geometry.components[i].clone())]);
                }
            } else {
                {{ module }}.layers.vector.addFeatures([admin_geom]);
            }
            // Zooming to the bounds.
            {{ module }}.map.zoomToExtent(admin_geom.geometry.getBounds());
            if ({{ module }}.is_point){
                {{ module }}.map.zoomTo({{ point_zoom }});
            }
        }
    });
})(django.jQuery);
{% endblock init_function %}

comment:7 Changed 2 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted
  • UI/UX unset

From code inspection, I'm convinced that the bug report is correct. All other JavaScript regexps use double backslashes.

Changed 2 years ago by aaugustin

comment:8 Changed 2 years ago by aaugustin

I'm attaching the patch (1 char). I don't have time to test this now.

comment:9 Changed 2 years ago by claudep

  • Resolution set to fixed
  • Status changed from reopened to closed

In [17760]:

Fixed #10625 -- Fixed a Javascript regex in openlayers.js. Thanks timlinux for the report and Aymeric Augustin for the patch.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.