Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

JFormField strips all chars from the id attribute that are not #1469

Closed
ramalama opened this Issue · 8 comments

4 participants

@ramalama

JFormField::getId($fieldId, $fieldName) does a preg_replace('#\W#', '_', $id) to replace "illeagal" characters. In fact the replacement breakes the field.

However this is imho overrestrictive, probably not needed at all. I don't see a reason why the id attribute shall be treated differently from other attributes like name.

In my case it strips out german special characters like üäö etc. That's unwanted cause it prevents the Form to be loaded properly.

Suggestion: Either delete the $id = preg_replace('#\W#', '_', $id); line or replace it by a more accurate code. Plus add a comment why the replacement is necessary at all.

@elinw

Could you submit code for that and possibly a test?

@ramalama

Yes I probably could, first some questions to clarify what to implement:

Who knows why this replace is done?
Is there a good reason why this field is treated differently?

@LouisLandry

A quick look at http://www.w3schools.com/tags/att_standard_id.asp shows that there are some restrictions on ID attribute valid characters. Additionally, while in some versions of HTML/XHTML accented characters as well as period . and colon : are allowed, those characters have proven to be problematic for several browsers over the years as well as some JavaScript frameworks/libraries. If we can assert that opening it up won't cause undue harm in those situations then I'd be happy for us to remove the restriction.

@ramalama

Thanks @LouisLandry for the information! So this is not a issue but by design to follow the rules of html.
Anyway the replacing was an unexpected behaviour for me: I would expect either an error thrown or just outputting the not html complainant element.

I'd suggest something like

if(!preg_match('#^[A-Za-z][A-Za-z0-9-_]$#', $id))
throw new Exception("The \$id:'$id' is invalid. According to w3schools.com the \$id must match the RegEx ^[A-Za-z][A-Za-z0-9-_]
$ .");

Any thoughts on this?

@elinw

I might put it in the docblock instead

@elinw

@ramalama Are you going to send a pull request for this?

@ramalama

@elinw I'm really sorry, but I dont have any time to spend on this at the moment.

@dongilbert
Collaborator

We do need to follow HTML restrictions, and it's easiest to just strip invalid characters. However, I can understand why you see it as a bug, since it was unexpected behavior. In my opinion, the CMS should have some sort of alert when filtering if the filtering results in data loss, as is the case here. Just to notify the user. However, this is implementation specific (meaning CMS) so I'm not sure it's something we need in the platform.

@dongilbert dongilbert closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.