.NET WPF doesn't render a simple card

[paulcam206]

.NET WPF (haven't tested HTML yet) fails to render this card:

{
	"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
	"type": "AdaptiveCard",
	"version": "1.2",
	"body": [
		{
			"type": "TextBlock",
			"id": "3",
			"text": "Container default style"
		}
	]
}

exception message:

System.ArgumentException
  HResult=0x80070057
  Message='3' is not a valid value for property 'Name'.
  Source=WindowsBase
  StackTrace:
   at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal)
   at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value)
   at AdaptiveCards.Rendering.Wpf.AdaptiveRenderContext.Render(AdaptiveTypedElement element)
   at AdaptiveCards.Rendering.Wpf.AdaptiveContainerRenderer.AddContainerElements(Grid uiContainer, IList`1 elements, AdaptiveRenderContext context)
   at AdaptiveCards.Rendering.Wpf.AdaptiveCardRenderer.RenderAdaptiveCardWrapper(AdaptiveCard card, AdaptiveRenderContext context)
   at AdaptiveCards.Rendering.AdaptiveElementRenderers`2.<>c__DisplayClass1_0`1.<Set>b__0(AdaptiveTypedElement typedElement, TContext tContext)
   at AdaptiveCards.Rendering.Wpf.AdaptiveRenderContext.Render(AdaptiveTypedElement element)
   at AdaptiveCards.Rendering.Wpf.AdaptiveCardRenderer.RenderCard(AdaptiveCard card)

Aside from the exception itself, it's noteworthy that we're not converting the System.ArgumentException to an AdaptiveException.

[paulcam206]

card renders as expected in designer...

[shalinijoshi19]

@paulcam206 does this need to be fixed for 1.2? If so we should get to this soon

[paulcam206]

@shalinijoshi19 -- this should be fixed with changes to what id property values we allow. I haven't investigated this issue yet, but I suspect that the WPF element's name property isn't allowed to be strictly numeric. I think we should keep this one to track.

[shalinijoshi19]

@paulcam206 still working on this for 1.2 or should we move out?

[paulcam206]

we should move this out. I took a look. it only affects .Net WPF and isn't a regression.

[almedina-ms]

Didn't we had to set rules on how the ids for elements had to be? I believe the issue has to do with the card having a number only id

[almedina-ms]

Exactly what Paul commented

[almedina-ms]

@paulcam206 @matthidinger what was the format we decided upon this??

According to the WPF documentation " a Name must start with a letter or the underscore character (_), and must contain only letters, digits, or underscores. "

[matthidinger]

We have 2 choices I think:

1. Update the schema to enforce a regex for ID similar to HTML4. (Counter data point: HTML5 is more permissive and does allow a single number)
   HTML4: ID and NAME 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 (".").
2. Update the WPF renderer to fixup/prefix all element Names with an underscore and leave the schema permissive.

I'm kind of on the fence personally. Does this payload work properly on all other renderers as of today? If so, I'm inclined to keep it permissive and do the WPF fixup.

@dclaux any thoughts?

[almedina-ms]

@matthidinger I only found WPF to have this issue, all the other platforms accept numbers as id

[shalinijoshi19]

1. What's the case where we expect a customer to specifier an 'id'? With option 2 above would it get confusing if they input something explicitly which was then changed from underneath them ? Do they try to reference it later in the card themselves at all? Because if not, i do agree Section "title" property #2 seems least breaking and targeted.

2. What is the extent of "permissiveness" defined today? Also the bug is not just numeric id's it's also special characters otherwise allowed in HTML.. Why is the ' _ ' character special? Eg of a card that doesnt work even though it doesnt have only numerics (eg 1: or _1: or a:)

{ "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", "type": "AdaptiveCard", "version": "1.2", "body": [ { "type": "Container", "items": [ { "type": "TextBlock", "text": "This is some of my new bug bash text", "id": "alpha" }, { "type": "Image", "url": "http://adaptivecards.io/content/cats/1.png", "id": "abd4", "altText": "This is a cat" } ] } ], **"id": "a:"** }

3. Also the error we are surfacing is confusing - the customer tries to set the "id" property but the error seems to reference a "name" property that doesnt seem very intuitive and confusing. We should fix that. @almedina-ms as part of this fix

4. There could be a 3rd option too (mix of 1+2 above from @matthidinger ) where we
   (a) support HTML5 style id's directly (without the need for "fixing up" under the covers ) for all renderers that dont currently today and also
   (b) do this "fixup" in WPF with the underscore prefix for consistency issue across renderers..

How expensive is 4 (a) going to be @almedina-ms ?

[almedina-ms]

@andrewleader proposal may still be the best option, having a dictionary when rendering with all the ids and rendered items to handle all id's , that way we don't have to worry about changing the ids prefixing an underscore '_' and we don't have to deal with the 'Name' message and the impact on the client side

Fixing it on WPF isn’t as trivial as just prepending an underscore (that doesn’t handle cases where there’s a space), however it is still relatively trivial to fix.

However, I think WPF needs to be updated regardless as the code as it’s written today poses a potential security threat to host apps. Today, we’re assigning the Id to the Name property of the actual WPF elements in the visual tree… what happens if the author uses the same Id as the host app is using for an element name elsewhere? If that author then looks through the visual tree for their named element, they might instead find the element from the author. Now, it’s not best practice to look up elements by name, as it requires iterating through the entire visual tree, so that’s probably a very unlikely vulnerability. However, I don’t think there’s any reason for us to be using the Name and the visual tree for this either. We should be storing a separate dictionary of Id->Element, which would be on the order of O(1) rather than O(n) for grabbing the element in order to toggle the visibility.

HTML5’s identity syntax sounds fine with me too

[almedina-ms]

1. What's the case where we expect a customer to specifier an 'id'? With option 2 above would it get confusing if they input something explicitly which was then changed from underneath them ? Do they try to reference it later in the card themselves at all? Because if not, i do agree Section "title" property #2 seems least breaking and targeted.

The users specify ids for retrieving the input data in submit actions and for defining the elements to be hidden by a ToggleVisibility action

2. What is the extent of "permissiveness" defined today? Also the bug is not just numeric id's it's also special characters otherwise allowed in HTML.. Why is the ' _ ' character special? Eg of a card that doesnt work even though it doesnt have only numerics (eg 1: or _1: or a:)

{ "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", "type": "AdaptiveCard", "version": "1.2", "body": [ { "type": "Container", "items": [ { "type": "TextBlock", "text": "This is some of my new bug bash text", "id": "alpha" }, { "type": "Image", "url": "http://adaptivecards.io/content/cats/1.png", "id": "abd4", "altText": "This is a cat" } ] } ], **"id": "a:"** }

We dont have any rules defined for how an id should be formed and we don't have tests on other type of scenarios so there could be other mismatches between the platforms.

Also the error we are surfacing is confusing - the customer tries to set the "id" property but the error seems to reference a "name" property that doesnt seem very intuitive and confusing. We should fix that. @almedina-ms as part of this fix

The error is confusing as we're setting the name property to the rendered UIElement for retrieving the element using the findElementInLogicTree(string name) function, Andrew's proposal was to create a different dictionary to avoid having to deal with the name property all together

There could be a 3rd option too (mix of 1+2 above from @matthidinger ) where we
(a) support HTML5 style id's directly (without the need for "fixing up" under the covers ) for all renderers that dont currently today and also
(b) do this "fixup" in WPF with the underscore prefix for consistency issue across renderers..
How expensive is 4 (a) going to be @almedina-ms ?

The draft #3242 supports any kind of string so its just as permissive as the string type is, it just requires an extra dictionary which contains pairs of string and UIElement where the string is the object model's id

[almedina-ms]

@shalinijoshi19 , any thoughts about this?

[shalinijoshi19]

Thanks! Can you do me a favor write this up into a one-pager and lets review this on Friday? Thanks

[shalinijoshi19]

@almedina-ms we'll need a onepager for this to review please! thanks

[ghost]

This issue has been automatically marked as stale because it has not had any activity for 5 days.

[ghost]

🎉AdaptiveCards@2020.08 has been released which fixes this issue.:tada:

Handy links:

• Release Notes
• AdaptiveCards Website
• Official NPM Designer Release
• Official NPM Templating Release
• Official NPM Renderer Release
• Official Nuget Templating Release
• Official Nuget Renderer Release
• Official Maven Renderer Release
• Official CocoaPod Renderer Release