dotnet / razor

Compiler and tooling experience for Razor ASP.NET Core apps in Visual Studio, Visual Studio for Mac, and VS Code.
https://asp.net
MIT License
499 stars 191 forks source link

EventHandlerAttribute: enableStopPropagation and enablePreventDefault are swapped #10497

Open FStolte opened 4 months ago

FStolte commented 4 months ago

Summary

The parameters enableStopPropagation and enablePreventDefault of the EventHandlerAttribute are swapped by the razor compiler, so that setting enableStopPropagation = true actually enables preventDefault, and vice versa.

Steps to reproduce

I added the following class to my project in order to add support for the "transitionend" event to blazor:

[EventHandler("ontransitionend", typeof(EventArgs), enableStopPropagation: true, enablePreventDefault: false)]
public static class EventHandlers { }

When I set @ontransitionend=SomeCallback on a div in one of my components, this is recognized immediately by the compiler, according to the syntax highlighting. However, when I try to set @ontransitionend:stopPropagation on another div, this is not recognized (the whole string was red like an html-attribute). When I then start my app, I get an exception complaining about a malformed string, i.e. the unprocessed "@" in the html-attribute name. Furthermore, adding @ontransitionend:preventDefault is recognized and handled by the compiler, when it should be disabled. Swapping the values for both parameters results in the exact opposite behavior.

Possible cause

https://github.com/dotnet/razor/blob/056b3bede2afd12405a60295513e09c5fbaa2d7f/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/EventHandlerTagHelperDescriptorProvider.cs#L74-L75 In EventHandlerTagHelperDescriptorProvider, the constructor arguments of the EventHandlerAttribute are accessed in the wrong order. The variable enablePreventDefault is being assigned the argument with index 2, when that actually is the enableStopPropagation parameter; the same goes for the enableStopPropagation variable being assigned the argument with index 3.

As far as I can see, this is the only place that accesses the values of these constructor arguments or the properties they are assigned to.

jjonescz commented 4 months ago

Thanks for reporting this issue. Looks like you found the cause correctly.

I guess no-one noticed this issue because most EventHandlers defined in aspnetcore use true for both: https://github.com/dotnet/aspnetcore/blob/99f2ba5fd1d668fecbff1e1a9ba6a541c99bec79/src/Components/Web/src/Web/EventHandlers.cs

DustinCampbell commented 2 months ago

I just noticed the bug while fixing nullability warnings in that code. :smile: