Opened 15 years ago

Closed 14 years ago

Last modified 12 years ago

#11810 closed (fixed)

[gis] Typo in OpenLayers Admin JS

Reported by: Robert Coup Owned by: jbronn
Component: GIS Version: dev
Severity: Keywords: admin gis osmgeoadmin
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There's a minor typo in the GIS admin openlayers javascript, for GeoAdmins where modifiable=False.

Attachments (3)

11810.patch (596 bytes ) - added by Robert Coup 15 years ago.
Against r11479
11810_v2.diff (2.0 KB ) - added by jbronn 15 years ago.
11810-r12932.diff (6.3 KB ) - added by Ramiro Morales 14 years ago.
Justin's patch modified to additionally tests if the field is in the new 1.2 readonly_fields ModelAdmin? option

Download all attachments as: .zip

Change History (11)

by Robert Coup, 15 years ago

Attachment: 11810.patch added

Against r11479

comment:1 by jbronn, 15 years ago

Keywords: gis osmgeoadmin added
milestone: 1.2
Owner: changed from nobody to jbronn

Obviously this is a bug. Unfortunately, fixing the typo just exposes further bugs -- sigh, thanks rcoup :) . For example, if you already have a feature and modifiable=False, then the following JS error occurs (I'm using the WorldBorders model from the tutorial):

geodjango_mpoly.map.getControlsByClass("OpenLayers.Control.ModifyFeature")[0] is undefined

This is because the control wasn't enabled in the first place (now that the typo is fixed) -- thus, some more work is needed. I've attached a patch that addresses this additional problem, but I want to think about it a while. Some questions to ponder: do we still want to allow drawing of new features if modifiable=False (which the new patch allows)? Should this be an option on OSMGeoAdmin at all when there's the editable Field option?

by jbronn, 15 years ago

Attachment: 11810_v2.diff added

comment:2 by jbronn, 15 years ago

Triage Stage: UnreviewedDesign decision needed

comment:3 by Robert Coup, 15 years ago

I do try :) I wanted modifiable=False to act like a read-only field: just show me the geometry on a map. So it actually worked fine with the typo, except for the JS error in the browser console.

I guess this gets back into the read-only-admin-fields issue (#342). Personally, read-only fields make a tonne of sense and even more so for map data. editable=False means "don't display this at all in the Admin", which isn't quite the same thing.

comment:4 by anonymous, 14 years ago

Version: SVN1.2-beta

comment:5 by anonymous, 14 years ago

Version: 1.2-betaSVN

by Ramiro Morales, 14 years ago

Attachment: 11810-r12932.diff added

Justin's patch modified to additionally tests if the field is in the new 1.2 readonly_fields ModelAdmin? option

comment:6 by jbronn, 14 years ago

Resolution: fixed
Status: newclosed

(In [12995]) Fixed #11810 -- Fixed typo and errors that prevented modifiable from working in the geographic admin. Thanks to Rob Coup for the bug report. Refs #12504.

comment:7 by jbronn, 14 years ago

(In [12996]) [1.1.X] Fixed #11810 -- Fixed typo and errors that prevented modifiable from working in the geographic admin. Thanks to Rob Coup for the bug report. Refs #12504.

Backport of r12995 from trunk.

comment:6 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top