CreateSanitizedId should change to reflect HTML5 latest spec

[doggy8088]

Is this a Bug or Feature request?:

Bug

Steps to reproduce (preferrably a link to a GitHub repo with a repro project):

Repo: https://github.com/doggy8088/ASPNETCORE_HTML5_Id

Steps:

1. git clone https://github.com/doggy8088/ASPNETCORE_HTML5_Id.git
2. cd ASPNETCORE_HTML5_Id/ASPNETCORE_HTML5_Id
3. dotnet run
4. Navigate to https://localhost:5001/Home/Test
5. Check the following id attribute:

Description of the problem:

Here is my model:

The HTML output become:

There is a CreateSanitizedId method in the Microsoft.AspNetCore.Mvc.Rendering.TagBuilder class. In this method, it sanitized all the id attribute for all HTML tags to apply HTML4.01 spec for a long time which is no problem at all.

The HTML 4.01 spec states that ID tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens (-), underscores (_), colons (:), and periods (.). HTML5 gets rid of that restrictions on the id attribute now.

For a modern web framework, I think it should align with the HTML5 specification and deprecated the old behavior.

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

All of them. All versions.

[mkArtakMSFT]

Thanks for contacting us, @doggy8088.
@dougbu can you please look into this? Thanks!

[dougbu]

Changes here are possible but not straightforward due to the need for a compatibility switch and the fact the methods involved are static.

@doggy8088 what are the practical downside(s) i.e. bug(s) that result from MVC not being as relaxed about id sanitization as HTML 5?

[doggy8088]

@dougbu If some Models defined non-ASCII characters as a property name, the @Html.Id() or @Html.IdFor() can't produce correct id in the HTML. If I'm using Entity Framework DB First, all the models are generated from the database. If there are any column name appear some non-ASCII characters , the ASP.NET MVC still can't produce the right id in the HTML. That's the main problem of this issue.

[rynowak]

If we're producing name values that are invalid today then this seems like something we could address without a compat switch.. How to produce them is another story, they would certainly not be semantically meaningful or easy to predict.

[dougbu]

@rynowak this is about id attribute values, not name values.

@doggy8088 what do you mean by "correct" or "right" id values -- something more like the corresponding name? The existing code meets HTML 4.01 requirements. The generated values are also valid according to HTML 5.

Again, what are the practical downside(s) i.e. bug(s) that result from MVC not being as relaxed about id sanitization as HTML 5?

[doggy8088]

@dougbu The right id value should be the same as name attribute. It shouldn't have any outdated restriction on it's characters. The current id is also valid for HTML5 for sure, but this will produce some errors I experienced before.

The main downside(s) of this issue is mainly for interacting with ASP.NET MVC Views and JavaScript. When we generate a list of input fields on the HTML and have to deal with it using JavaScript. That will be a big problem just because all the ids are been sanitized. Especially for dynamic columns/fields.

[dougbu]

will produce some errors I experienced before.

Please describe the errors you've experienced. Are you for example seeing multiple names sanitized to identical ids, which would violate id uniqueness requirements?

have to deal with it using JavaScript

Not quite sure what you mean here. Following associations between elements (e.g. an <input/> and its <label>) should not require regenerating the ids.

[doggy8088]

@dougbu Yes, sometimes we need id for uniqueness.

I'm not telling about the <input/> and <label> issue. It's sometimes we need calling getElementByID() method to selecting the right field so the id is important in this circumstances.

[dougbu]

🆗 we either need to ensure distinct name values result in distinct id values e.g. add a unique suffix when collisions occur or relax the restrictions to allow many more characters in ids. Either way, the change must be done behind a compatibility switch.

[javiercn]

I think this bug is simply about not sanitizing the id element (as HTML5 allows everything but spaces) so that it matches the property name when it contains non unicode characters and so that its exactly the same as the property name for purposes like interacting with the element through JavaScript.

I'm not sure what transformations we do when we find non-ascii characters on the id, but if its not obvious, it clearly posses an issue when interacting through JavaScript.

In theory we could do this, I don't think we need a compat switch for it, but just to announce the breaking change. We would have a switch to turn the old behavior back on, but you would be required to turn that on when you update (so as to not cause a big disruption for people using ids in their current javascript/css).

Any obscure transformation we can avoid is a good thing, but we need to preserve this behavior to not disrupt existing apps.

@rynowak Are you ok if we update here and provide a switch to go back to the old behavior? I think this leaves us in a better position moving forward.

[dougbu]

@javiercn

we would have a switch to turn the old behavior back on

That sounds like a compat switch because I'm not sure why we'd preserve it going forward.

I suspect we also need to change Microsoft.jQuery.Unobtrusive.Validation to support not doing the same transformations we've been doing forever. The hardest part will likely be linking that JavaScript switch to the compat switch. In any case, the JavaScript switch needs to preserve the existing behaviour by default because users may update the package before they update anything from this repo.

[javiercn]

@dougbu We would like to support older browsers where we can and give people some room to update. You are right in that we would have to update the jquery bits.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[mkArtakMSFT]

Hi. Thanks for contacting us.
We're closing this issue as there was not much community interest in this ask for quite a while now.
You can learn more about our triage process and how we handle issues by reading our Triage Process writeup.