JavaScript error: TypeError: features is undefined [#970362]

When invalid WKT data is supplied, there is a JavaScript error that causes the map to be replaced by the message: "Error during map rendering: TypeError: features is undefined".

This error can be avoided by checking that wktFormat.read() returns a non-null value.

Please see the attached patch.

Comment	File	Size	Author
	openlayers-fix_wktread_invalid_error.patch	1.65 KB	arithmetric

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

zzolo CreditAttribution: zzolo commented 13 November 2010 at 13:44

Status:

Needs review

» Fixed

Thanks @arithmetric. Committed: http://drupal.org/cvs?commit=450258

Comment #2

strk CreditAttribution: strk commented 15 November 2010 at 15:03

Status:

Fixed

» Needs work

I actually still get an error when typing wrong WKT:

Error during map rendering: TypeError: newFeature is undefined

The above by adding a final ",FAKE" component to the GEOMETRYCOLLECTION..

Note that doing so made the node in a state in which I can NOT edit it anymore, the only
way to get the node in a usable state again is manual editing trough the WKT field.

Comment #3

tmcw CreditAttribution: tmcw commented 15 November 2010 at 15:09

How can that not be expected/ideal behavior? Unless the OpenLayers project implements WKT parsing error reporting, or this module implements some hack to let invalid WKT be parsed or something, that's how things will be. And the level of knowledge required to manually edit WKT is equivalent to the level of knowledge to fix errors with your WKT, right?

Comment #4

strk CreditAttribution: strk commented 15 November 2010 at 16:46

Well, for once I woudn't have wanted the invalid WKT go into the database.
Dunno if this is different from the original report, anyway it it what disturbed me: "lemme try this WKT -> gone into db -> unable to render"

Comment #5

tmcw CreditAttribution: tmcw commented 15 November 2010 at 16:53

Still, that really moves the purpose of editing raw WKT around. If you put in bad data, what should happen? Should there be some kind of roundtrip of greying out the save button while things are invalid? Or something like that? Anyway, this seems like feature creep for a class of user that doesn't require that much hand-holding, and it'll lead to a brittle, overly complex implementation. After all, editors let you save invalid markup, code, etc. And making OpenLayers validate the WKT before saving also means that you're restricting the definition of WKT to OpenLayers's, and parsing even more often. All in all, I think that this just shouldn't be a priority: users who know WKT markup can use it, otherwise they should use the UI, and be protected from errors like this one.

Comment #6

strk CreditAttribution: strk commented 15 November 2010 at 17:08

If you put in bad data, what should happen?

The form validation should fail, the offending input be highlighted, a warning message shown on top
and you'd still be in the "edit" page. Pretty much what happens when you put invalid input in any other field...

WKT is very well defined, so I think "restricting to openlayers view of it" is a false problem.
It's OpenLayersWKT, btw, so even if openlayers only supported a subset of WKT, I still wouldnt' have a problem with
aforementioned cck type to refuse unsupported-by-openlayers input.

Comment #7

arithmetric CreditAttribution: arithmetric commented 15 November 2010 at 22:29

@zzolo: Thanks for the commit.

@strk: Regarding the error you mentioned in #2 ("TypeError: newFeature is undefined"), I believe you have found another spot where an error could be caught. This message refers to a different variable than in my original report.

As for WKT validation, it would be a neat feature, but it is beyond the scope of this bug report. Even if WKT input was validated, I would still consider it a bug if a preventable JavaScript error disables map functionality.