Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Blazor] Fix WebView renderer not correctly dispatching browser events #49958

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

MackinnonBuck
Copy link
Member

Fix WebView renderer not correctly dispatching browser events

Fixes an issue where Blazor Hybrid throws an exception when a browser event gets emitted. This results in console errors upon user interaction.

Fixes dotnet/maui/issues/16609

@MackinnonBuck MackinnonBuck requested a review from a team as a code owner August 9, 2023 18:24
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Aug 9, 2023
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Have we verified there aren't regressions in other APIs that fallback to RenderId=0?

@MackinnonBuck
Copy link
Member Author

@captainsafia Hopefully, all cases that fallback to a renderer ID of 0 have been eliminated. I've scanned the source for remaining cases, but it looks like they should all be addressed now. The root of the problem was that for one method, rendererId was optional when it should have been required. Had it been required, the TypeScript compiler would have caught the bug earlier.

Copy link
Member

@Eilon Eilon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I literally watched you make the fix, so I approve 😁

@MackinnonBuck MackinnonBuck merged commit c48e7a4 into main Aug 9, 2023
26 checks passed
@MackinnonBuck MackinnonBuck deleted the mbuck/blazor-hybrid-fix branch August 9, 2023 20:55
@ghost ghost added this to the 8.0-rc1 milestone Aug 9, 2023
@@ -24,7 +24,7 @@ export function attachRootComponentToLogicalElement(browserRendererId: number, l
browserRenderer.attachRootComponentToLogicalElement(componentId, logicalElement, appendContent);
}

export function attachRootComponentToElement(elementSelector: string, componentId: number, browserRendererId?: number): void {
export function attachRootComponentToElement(elementSelector: string, componentId: number, browserRendererId: number): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since browserRenderId is now required could the fallback to 0 in line 47 be removed?
browserRendererId || 0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it could be. I can remove it the next time I touch this area, but feel free to open a PR if you feel inclined :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
4 participants