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

Fix "no event handler" in simultaneous blur+removal case #31612

Merged
merged 5 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ internal class WebAssemblyRenderer : Renderer
{
private readonly ILogger _logger;
private readonly int _webAssemblyRendererId;
private readonly QueueWithLast<IncomingEventInfo> deferredIncomingEvents = new();

private bool isDispatchingEvent;
private Queue<IncomingEventInfo> deferredIncomingEvents = new Queue<IncomingEventInfo>();

/// <summary>
/// Constructs an instance of <see cref="WebAssemblyRenderer"/>.
Expand Down Expand Up @@ -103,7 +103,23 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch)
_webAssemblyRendererId,
batch);

return Task.CompletedTask;
if (deferredIncomingEvents.Count == 0)
{
// In the vast majority of cases, since the call to update the UI is synchronous,
// we just return a pre-completed task from here.
return Task.CompletedTask;
}
else
{
// However, in the rare case where JS sent us any event notifications that we had to
// defer until later, we behave as if the renderbatch isn't acknowledged until we have at
// least dispatched those event calls. This is to make the WebAssembly behavior more
// consistent with the Server behavior, which receives batch acknowledgements asynchronously
// and they are queued up with any other calls from JS such as event calls. If we didn't
// do this, then the order of execution could be inconsistent with Server, and in fact
// leads to a specific bug: https://github.com/dotnet/aspnetcore/issues/26838
return deferredIncomingEvents.Last.StartHandlerCompletionSource.Task;
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -144,7 +160,7 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev
{
var info = new IncomingEventInfo(eventHandlerId, eventFieldInfo, eventArgs);
deferredIncomingEvents.Enqueue(info);
return info.TaskCompletionSource.Task;
return info.FinishHandlerCompletionSource.Task;
}
else
{
Expand All @@ -171,16 +187,20 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev
private async Task ProcessNextDeferredEventAsync()
{
var info = deferredIncomingEvents.Dequeue();
var taskCompletionSource = info.TaskCompletionSource;

try
{
await DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs);
taskCompletionSource.SetResult();
var handlerTask = DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs);
info.StartHandlerCompletionSource.SetResult();
await handlerTask;
info.FinishHandlerCompletionSource.SetResult();
}
catch (Exception ex)
{
taskCompletionSource.SetException(ex);
// Even if the handler threw synchronously, we at least started processing, so always complete successfully
info.StartHandlerCompletionSource.TrySetResult();

info.FinishHandlerCompletionSource.SetException(ex);
}
}

Expand All @@ -189,14 +209,16 @@ readonly struct IncomingEventInfo
public readonly ulong EventHandlerId;
public readonly EventFieldInfo? EventFieldInfo;
public readonly EventArgs EventArgs;
public readonly TaskCompletionSource TaskCompletionSource;
public readonly TaskCompletionSource StartHandlerCompletionSource;
public readonly TaskCompletionSource FinishHandlerCompletionSource;

public IncomingEventInfo(ulong eventHandlerId, EventFieldInfo? eventFieldInfo, EventArgs eventArgs)
{
EventHandlerId = eventHandlerId;
EventFieldInfo = eventFieldInfo;
EventArgs = eventArgs;
TaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
StartHandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
FinishHandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
}
}

Expand Down Expand Up @@ -225,5 +247,30 @@ public static void UnhandledExceptionRenderingComponent(ILogger logger, Exceptio
exception);
}
}

private class QueueWithLast<T>
{
private readonly Queue<T> _items = new();

public int Count => _items.Count;

public T? Last { get; private set; }

public T Dequeue()
{
if (_items.Count == 1)
{
Last = default;
}

return _items.Dequeue();
}

public void Enqueue(T item)
{
Last = item;
_items.Enqueue(item);
}
}
}
}
11 changes: 11 additions & 0 deletions src/Components/test/E2ETest/Tests/EventTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ public void FocusEvents_CanTrigger()
Browser.Equal("onfocus,onfocusin,onblur,onfocusout,", () => output.Text);
}

[Fact]
public void FocusEvents_CanReceiveBlurCausedByElementRemoval()
{
// Represents https://github.com/dotnet/aspnetcore/issues/26838

Browser.MountTestComponent<FocusEventComponent>();

Browser.FindElement(By.Id("button-that-disappears")).Click();
Browser.Equal("True", () => Browser.FindElement(By.Id("button-received-focus-out")).Text);
}

[Fact]
public void MouseOverAndMouseOut_CanTrigger()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<h2>Focus and activation</h2>

<p @onfocusin="OnFocusIn" @onfocusout="OnFocusOut">
Input: <input id="input" type="text" @onfocus="OnFocus" @onblur="OnBlur"/>
Input: <input id="input" type="text" @onfocus="OnFocus" @onblur="OnBlur" />
</p>
<p>
Output: <span id="output">@message</span>
Expand All @@ -12,40 +12,59 @@
<button @onclick="Clear">Clear</button>
</p>

<p>
A button that disappears when clicked:
@if (showButtonThatDisappearsWhenClicked)
{
<button id="button-that-disappears" @onfocusout="DisappearingButtonFocusOut" @onclick="MakeButtonDisappear">
Click me
</button>
}

Received focus out: <strong id="button-received-focus-out">@buttonReceivedFocusOut</strong>
</p>

<p>
Another input (to distract you) <input id="other" />
</p>

@code {

bool showButtonThatDisappearsWhenClicked = true;
bool buttonReceivedFocusOut;
string message;

void OnFocus(FocusEventArgs e)
{
message += "onfocus,";
StateHasChanged();
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing these StateHasChanged calls is a bit of unrelated cleanup. They're just not needed.

}

void OnBlur(FocusEventArgs e)
{
message += "onblur,";
StateHasChanged();
}

void OnFocusIn(FocusEventArgs e)
{
message += "onfocusin,";
StateHasChanged();
}

void OnFocusOut(FocusEventArgs e)
{
message += "onfocusout,";
StateHasChanged();
}

void Clear()
{
message = string.Empty;
}

void MakeButtonDisappear()
{
showButtonThatDisappearsWhenClicked = false;
}

void DisappearingButtonFocusOut()
{
buttonReceivedFocusOut = true;
}
}