Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Remove ~/ handling from core Razor. #427

Closed
Eilon opened this issue Jun 22, 2015 · 13 comments
Closed

Remove ~/ handling from core Razor. #427

Eilon opened this issue Jun 22, 2015 · 13 comments
Assignees
@NTaylorMullen
Labels
3 - Done enhancement
Milestone
1.0.0-beta7

Comments

@Eilon
Copy link
Member

Eilon commented Jun 22, 2015

Kind of related to #417

Allegedly Razor will resolve paths of all attributes of all HTML attributes, which means that ~'s will be stripped out regardless of whether the HTML (or Tag Helper) attribute represents a path or URL.

We should consider adding some new TagHelper attribute such as [UrlProperty] to indicate that Razor should code-gen path resolution code for the property.
@NTaylorMullen NTaylorMullen mentioned this issue Jun 22, 2015
Closed
@grahamehorner
Copy link

I agree however razor is also striping non-path attributes data-attrib="somedata~" and this may be by design in razor for paths; but what if a tag requires the ~ and it's not a path. I think TagHelpers shouldn't have razor pre-process the tag attributes other than the know HTML attributes that are path related.

@Eilon
Copy link
Member Author

Eilon commented Jun 23, 2015

@grahamehorner right, this bug is exactly to stop processing ~ in places where it's not a URL. Only href and src and perhaps some other specific HTML attributes should have URL processing.

@danroth27 danroth27 added this to the 4.0.0-beta7 milestone Jun 24, 2015
@danroth27 danroth27 modified the milestones: 4.0.0-beta7, 4.0.0-beta6 Jul 6, 2015
@NTaylorMullen
Copy link
Contributor

Attributes that I've found with URL support based on the HTML5 spec:

  • href : <a>, <area>, <link>, <base>
  • src : <audio>, <embed>, <iframe>, <img>, <input>, <script>, <source>, <track>, <video>
  • action : <form>
  • cite : <blockquote>, <del>, <ins>, <q>
  • data : <object>
  • formaction : <button>, <input>
  • icon : <menuitem>
  • itemid : everything
  • manifest : <html>
  • poster : <video>
  • archive : <object>, <applet>

Discussing a right approach for this. There's a lot more attribute element combos then initially thought 😄

@NTaylorMullen
Copy link
Contributor

Talked with @DamianEdwards and since this capability is entirely dependent on which version of HTML you're using (HTML5, HTML4 etc.) we've decided to move this behavior from the core Razor parser into an auto-added TagHelper that exists in Microsoft.AspNet.Mvc.Razor.

The TagHelper will be responsible for consuming the various attribute combinations listed above and normalizing the URLs. To prevent excess editor highlighting I've created #447 to allow this TagHelper to add default Razor functionality.

@NTaylorMullen
Copy link
Contributor

Starting work on preliminary issues that will be required for this. In the end this issue will most likely be: remove ~ functionality from core Razor.

@NTaylorMullen NTaylorMullen mentioned this issue Jul 10, 2015
Closed
@grahamehorner
Copy link

@NTaylorMullen @DamianEdwards sounds great; happy to help out in reviewing and testing

@grahamehorner
Copy link

auto-added TagHelper; would this be a TagHelper that is registered with dependency injection and therefore a developer can override the default by registering the custom implementation?

@Eilon
Copy link
Member Author

Eilon commented Jul 10, 2015

I believe anything with tag helpers can be overridden. It's part of the design of the feature 😄

@grahamehorner
Copy link

@Eilon wasn't clear to me given the wording 'auto-added TagHelper' if this can be replaced by a custom TagHelper using config or dependency injection.

@Eilon
Copy link
Member Author

Eilon commented Jul 10, 2015

I think what they mean is that it'll just be there by default in the project template. Not forced.

@NTaylorMullen
Copy link
Contributor

@Eilon the idea is that it'd be auto-included in your views much like HtmlHelper is auto @injected into your view. The difference between the TagHelper to enable URL resolution and others is that it wont require an initial @addTagHelper because it'll be added by default. If you don't want the TagHelper you'll be able to @removeTagHelper "*, Microsoft.AspNet.Mvc.Razor".

The thought behind the auto-adding approach is that not everyone will want to use TagHelpers in their project so auto-@addTagHelpering a helper TagHelper will keep the experience they were used to in MVC5 but allow the ability to opt-out if need be.

@Eilon
Copy link
Member Author

Eilon commented Jul 11, 2015

Got it.

@danroth27 danroth27 modified the milestones: 4.0.0-beta7, 4.0.0-beta6 Jul 16, 2015
NTaylorMullen added a commit that referenced this issue Jul 28, 2015
@NTaylorMullen
Remove ~/ handling from core Razor.
39ec988 
- Removed parsing, chunk generation and code generation.
- Removed related tests.
- Did not modify existing unrelated tests with ~/ since it's just plain text and may be valid for end-users.

#427
@NTaylorMullen NTaylorMullen mentioned this issue Jul 28, 2015
Merged
NTaylorMullen added a commit that referenced this issue Jul 31, 2015
@NTaylorMullen
Remove ~/ handling from core Razor.
48a6b56 
- Removed parsing, chunk generation and code generation.
- Removed related tests.
- Did not modify existing unrelated tests with ~/ since it's just plain text and may be valid for end-users.

#427
@NTaylorMullen
Copy link
Contributor

48a6b56

@NTaylorMullen NTaylorMullen changed the title Restrict '~' handling to only 'src' and 'href' attributes of HTML elements Remove ~/ handling from core Razor. Aug 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NTaylorMullen NTaylorMullen

Labels
3 - Done enhancement
Projects
None yet
Milestone
1.0.0-beta7
Development

No branches or pull requests
4 participants
@Eilon @danroth27 @NTaylorMullen @grahamehorner