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

For two-way bindings, enforce consistency between .NET model and DOM by patching old tree. Fixes #8204 #11438

Merged
merged 4 commits into from
Jun 24, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public partial class WebAssemblyRenderer : Microsoft.AspNetCore.Components.Rende
public WebAssemblyRenderer(System.IServiceProvider serviceProvider) : base (default(System.IServiceProvider)) { }
public System.Threading.Tasks.Task AddComponentAsync(System.Type componentType, string domElementSelector) { throw null; }
public System.Threading.Tasks.Task AddComponentAsync<TComponent>(string domElementSelector) where TComponent : Microsoft.AspNetCore.Components.IComponent { throw null; }
public override System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; }
public override System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo eventFieldInfo, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; }
protected override void Dispose(bool disposing) { }
protected override void HandleException(System.Exception exception) { }
protected override System.Threading.Tasks.Task UpdateDisplayAsync(in Microsoft.AspNetCore.Components.Rendering.RenderBatch batch) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ protected override void HandleException(Exception exception)
}

/// <inheritdoc />
public override Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArgs)
public override Task DispatchEventAsync(int eventHandlerId, EventFieldInfo eventFieldInfo, UIEventArgs eventArgs)
{
// Be sure we only run one event handler at once. Although they couldn't run
// simultaneously anyway (there's only one thread), they could run nested on
Expand All @@ -135,7 +135,7 @@ public override Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArg

if (isDispatchingEvent)
{
var info = new IncomingEventInfo(eventHandlerId, eventArgs);
var info = new IncomingEventInfo(eventHandlerId, eventFieldInfo, eventArgs);
deferredIncomingEvents.Enqueue(info);
return info.TaskCompletionSource.Task;
}
Expand All @@ -144,7 +144,7 @@ public override Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArg
try
{
isDispatchingEvent = true;
return base.DispatchEventAsync(eventHandlerId, eventArgs);
return base.DispatchEventAsync(eventHandlerId, eventFieldInfo, eventArgs);
}
finally
{
Expand All @@ -168,7 +168,7 @@ private async Task ProcessNextDeferredEventAsync()

try
{
await DispatchEventAsync(info.EventHandlerId, info.EventArgs);
await DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs);
taskCompletionSource.SetResult(null);
}
catch (Exception ex)
Expand All @@ -180,12 +180,14 @@ private async Task ProcessNextDeferredEventAsync()
readonly struct IncomingEventInfo
{
public readonly int EventHandlerId;
public readonly EventFieldInfo EventFieldInfo;
public readonly UIEventArgs EventArgs;
public readonly TaskCompletionSource<object> TaskCompletionSource;

public IncomingEventInfo(int eventHandlerId, UIEventArgs eventArgs)
public IncomingEventInfo(int eventHandlerId, EventFieldInfo eventFieldInfo, UIEventArgs eventArgs)
{
EventHandlerId = eventHandlerId;
EventFieldInfo = eventFieldInfo;
EventArgs = eventArgs;
TaskCompletionSource = new TaskCompletionSource<object>();
}
Expand Down
6 changes: 3 additions & 3 deletions src/Components/Browser.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

Large diffs are not rendered by default.

36 changes: 19 additions & 17 deletions src/Components/Browser.JS/src/Rendering/BrowserRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EventDelegator } from './EventDelegator';
import { EventForDotNet, UIEventArgs } from './EventForDotNet';
import { LogicalElement, PermutationListEntry, toLogicalElement, insertLogicalChild, removeLogicalChild, getLogicalParent, getLogicalChild, createAndInsertLogicalContainer, isSvgElement, getLogicalChildrenArray, getLogicalSiblingEnd, permuteLogicalChildren, getClosestDomElement } from './LogicalElements';
import { applyCaptureIdToElement } from './ElementReferenceCapture';
import { EventFieldInfo } from './EventFieldInfo';
const selectValuePropname = '_blazorSelectValue';
const sharedTemplateElemForParsing = document.createElement('template');
const sharedSvgElemForParsing = document.createElementNS('http://www.w3.org/2000/svg', 'g');
Expand All @@ -18,8 +19,8 @@ export class BrowserRenderer {

public constructor(browserRendererId: number) {
this.browserRendererId = browserRendererId;
this.eventDelegator = new EventDelegator((event, eventHandlerId, eventArgs) => {
raiseEvent(event, this.browserRendererId, eventHandlerId, eventArgs);
this.eventDelegator = new EventDelegator((event, eventHandlerId, eventArgs, eventFieldInfo) => {
raiseEvent(event, this.browserRendererId, eventHandlerId, eventArgs, eventFieldInfo);
});
}

Expand Down Expand Up @@ -50,7 +51,7 @@ export class BrowserRenderer {
const ownerDocument = getClosestDomElement(element).ownerDocument;
const activeElementBefore = ownerDocument && ownerDocument.activeElement;

this.applyEdits(batch, element, 0, edits, referenceFrames);
this.applyEdits(batch, componentId, element, 0, edits, referenceFrames);

// Try to restore focus in case it was lost due to an element move
if ((activeElementBefore instanceof HTMLElement) && ownerDocument && ownerDocument.activeElement !== activeElementBefore) {
Expand All @@ -70,7 +71,7 @@ export class BrowserRenderer {
this.childComponentLocations[componentId] = element;
}

private applyEdits(batch: RenderBatch, parent: LogicalElement, childIndex: number, edits: ArraySegment<RenderTreeEdit>, referenceFrames: ArrayValues<RenderTreeFrame>) {
private applyEdits(batch: RenderBatch, componentId: number, parent: LogicalElement, childIndex: number, edits: ArraySegment<RenderTreeEdit>, referenceFrames: ArrayValues<RenderTreeFrame>) {
let currentDepth = 0;
let childIndexAtCurrentDepth = childIndex;
let permutationList: PermutationListEntry[] | undefined;
Expand All @@ -91,7 +92,7 @@ export class BrowserRenderer {
const frameIndex = editReader.newTreeIndex(edit);
const frame = batch.referenceFramesEntry(referenceFrames, frameIndex);
const siblingIndex = editReader.siblingIndex(edit);
this.insertFrame(batch, parent, childIndexAtCurrentDepth + siblingIndex, referenceFrames, frame, frameIndex);
this.insertFrame(batch, componentId, parent, childIndexAtCurrentDepth + siblingIndex, referenceFrames, frame, frameIndex);
break;
}
case EditType.removeFrame: {
Expand All @@ -105,7 +106,7 @@ export class BrowserRenderer {
const siblingIndex = editReader.siblingIndex(edit);
const element = getLogicalChild(parent, childIndexAtCurrentDepth + siblingIndex);
if (element instanceof Element) {
this.applyAttribute(batch, element, frame);
this.applyAttribute(batch, componentId, element, frame);
} else {
throw new Error('Cannot set attribute on non-element child');
}
Expand Down Expand Up @@ -182,12 +183,12 @@ export class BrowserRenderer {
}
}

private insertFrame(batch: RenderBatch, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, frame: RenderTreeFrame, frameIndex: number): number {
private insertFrame(batch: RenderBatch, componentId: number, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, frame: RenderTreeFrame, frameIndex: number): number {
const frameReader = batch.frameReader;
const frameType = frameReader.frameType(frame);
switch (frameType) {
case FrameType.element:
this.insertElement(batch, parent, childIndex, frames, frame, frameIndex);
this.insertElement(batch, componentId, parent, childIndex, frames, frame, frameIndex);
return 1;
case FrameType.text:
this.insertText(batch, parent, childIndex, frame);
Expand All @@ -198,7 +199,7 @@ export class BrowserRenderer {
this.insertComponent(batch, parent, childIndex, frame);
return 1;
case FrameType.region:
return this.insertFrameRange(batch, parent, childIndex, frames, frameIndex + 1, frameIndex + frameReader.subtreeLength(frame));
return this.insertFrameRange(batch, componentId, parent, childIndex, frames, frameIndex + 1, frameIndex + frameReader.subtreeLength(frame));
case FrameType.elementReferenceCapture:
if (parent instanceof Element) {
applyCaptureIdToElement(parent, frameReader.elementReferenceCaptureId(frame)!);
Expand All @@ -215,7 +216,7 @@ export class BrowserRenderer {
}
}

private insertElement(batch: RenderBatch, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, frame: RenderTreeFrame, frameIndex: number) {
private insertElement(batch: RenderBatch, componentId: number, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, frame: RenderTreeFrame, frameIndex: number) {
const frameReader = batch.frameReader;
const tagName = frameReader.elementName(frame)!;
const newDomElementRaw = tagName === 'svg' || isSvgElement(parent) ?
Expand All @@ -229,11 +230,11 @@ export class BrowserRenderer {
for (let descendantIndex = frameIndex + 1; descendantIndex < descendantsEndIndexExcl; descendantIndex++) {
const descendantFrame = batch.referenceFramesEntry(frames, descendantIndex);
if (frameReader.frameType(descendantFrame) === FrameType.attribute) {
this.applyAttribute(batch, newDomElementRaw, descendantFrame);
this.applyAttribute(batch, componentId, newDomElementRaw, descendantFrame);
} else {
// As soon as we see a non-attribute child, all the subsequent child frames are
// not attributes, so bail out and insert the remnants recursively
this.insertFrameRange(batch, newElement, 0, frames, descendantIndex, descendantsEndIndexExcl);
this.insertFrameRange(batch, componentId, newElement, 0, frames, descendantIndex, descendantsEndIndexExcl);
break;
}
}
Expand Down Expand Up @@ -265,7 +266,7 @@ export class BrowserRenderer {
}
}

private applyAttribute(batch: RenderBatch, toDomElement: Element, attributeFrame: RenderTreeFrame) {
private applyAttribute(batch: RenderBatch, componentId: number, toDomElement: Element, attributeFrame: RenderTreeFrame) {
const frameReader = batch.frameReader;
const attributeName = frameReader.attributeName(attributeFrame)!;
const browserRendererId = this.browserRendererId;
Expand All @@ -277,7 +278,7 @@ export class BrowserRenderer {
if (firstTwoChars !== 'on' || !eventName) {
throw new Error(`Attribute has nonzero event handler ID, but attribute name '${attributeName}' does not start with 'on'.`);
}
this.eventDelegator.setListener(toDomElement, eventName, eventHandlerId);
this.eventDelegator.setListener(toDomElement, eventName, eventHandlerId, componentId);
return;
}

Expand Down Expand Up @@ -352,11 +353,11 @@ export class BrowserRenderer {
}
}

private insertFrameRange(batch: RenderBatch, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, startIndex: number, endIndexExcl: number): number {
private insertFrameRange(batch: RenderBatch, componentId: number, parent: LogicalElement, childIndex: number, frames: ArrayValues<RenderTreeFrame>, startIndex: number, endIndexExcl: number): number {
const origChildIndex = childIndex;
for (let index = startIndex; index < endIndexExcl; index++) {
const frame = batch.referenceFramesEntry(frames, index);
const numChildrenInserted = this.insertFrame(batch, parent, childIndex, frames, frame, index);
const numChildrenInserted = this.insertFrame(batch, componentId, parent, childIndex, frames, frame, index);
childIndex += numChildrenInserted;

// Skip over any descendants, since they are already dealt with recursively
Expand Down Expand Up @@ -397,7 +398,7 @@ function countDescendantFrames(batch: RenderBatch, frame: RenderTreeFrame): numb
}
}

function raiseEvent(event: Event, browserRendererId: number, eventHandlerId: number, eventArgs: EventForDotNet<UIEventArgs>) {
function raiseEvent(event: Event, browserRendererId: number, eventHandlerId: number, eventArgs: EventForDotNet<UIEventArgs>, eventFieldInfo: EventFieldInfo | null) {
if (preventDefaultEvents[event.type]) {
event.preventDefault();
}
Expand All @@ -406,6 +407,7 @@ function raiseEvent(event: Event, browserRendererId: number, eventHandlerId: num
browserRendererId,
eventHandlerId,
eventArgsType: eventArgs.type,
eventFieldInfo: eventFieldInfo,
};

return DotNet.invokeMethodAsync(
Expand Down
17 changes: 12 additions & 5 deletions src/Components/Browser.JS/src/Rendering/EventDelegator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { EventForDotNet, UIEventArgs } from './EventForDotNet';
import { EventFieldInfo } from './EventFieldInfo';

const nonBubblingEvents = toLookup([
'abort',
Expand All @@ -21,7 +22,7 @@ const nonBubblingEvents = toLookup([
]);

export interface OnEventCallback {
(event: Event, eventHandlerId: number, eventArgs: EventForDotNet<UIEventArgs>): void;
(event: Event, eventHandlerId: number, eventArgs: EventForDotNet<UIEventArgs>, eventFieldInfo: EventFieldInfo | null): void;
}

// Responsible for adding/removing the eventInfo on an expando property on DOM elements, and
Expand All @@ -40,7 +41,7 @@ export class EventDelegator {
this.eventInfoStore = new EventInfoStore(this.onGlobalEvent.bind(this));
}

public setListener(element: Element, eventName: string, eventHandlerId: number) {
public setListener(element: Element, eventName: string, eventHandlerId: number, renderingComponentId: number) {
// Ensure we have a place to store event info for this element
let infoForElement: EventHandlerInfosForElement = element[this.eventsCollectionKey];
if (!infoForElement) {
Expand All @@ -53,7 +54,7 @@ export class EventDelegator {
this.eventInfoStore.update(oldInfo.eventHandlerId, eventHandlerId);
} else {
// Go through the whole flow which might involve registering a new global handler
const newInfo = { element, eventName, eventHandlerId };
const newInfo = { element, eventName, eventHandlerId, renderingComponentId };
this.eventInfoStore.add(newInfo);
infoForElement[eventName] = newInfo;
}
Expand Down Expand Up @@ -89,15 +90,16 @@ export class EventDelegator {
const eventIsNonBubbling = nonBubblingEvents.hasOwnProperty(evt.type);
while (candidateElement) {
if (candidateElement.hasOwnProperty(this.eventsCollectionKey)) {
const handlerInfos = candidateElement[this.eventsCollectionKey];
const handlerInfos: EventHandlerInfosForElement = candidateElement[this.eventsCollectionKey];
if (handlerInfos.hasOwnProperty(evt.type)) {
// We are going to raise an event for this element, so prepare info needed by the .NET code
if (!eventArgs) {
eventArgs = EventForDotNet.fromDOMEvent(evt);
}

const handlerInfo = handlerInfos[evt.type];
this.onEvent(evt, handlerInfo.eventHandlerId, eventArgs);
const eventFieldInfo = EventFieldInfo.fromEvent(handlerInfo.renderingComponentId, evt);
this.onEvent(evt, handlerInfo.eventHandlerId, eventArgs, eventFieldInfo);
}
}

Expand Down Expand Up @@ -180,6 +182,11 @@ interface EventHandlerInfo {
element: Element;
eventName: string;
eventHandlerId: number;

// The component whose tree includes the event handler attribute frame, *not* necessarily the
// same component that will be re-rendered after the event is handled (since we re-render the
// component that supplied the delegate, not the one that rendered the event handler frame)
renderingComponentId: number;
}

function toLookup(items: string[]): { [key: string]: boolean } {
Expand Down
34 changes: 34 additions & 0 deletions src/Components/Browser.JS/src/Rendering/EventFieldInfo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
export class EventFieldInfo {
constructor(public componentId: number, public fieldValue: string | boolean) {
}

public static fromEvent(componentId: number, event: Event): EventFieldInfo | null {
const elem = event.target;
if (elem instanceof Element) {
const fieldData = getFormFieldData(elem);
if (fieldData) {
return new EventFieldInfo(componentId, fieldData.value);
}
}

// This event isn't happening on a form field that we can reverse-map back to some incoming attribute
return null;
}
}

function getFormFieldData(elem: Element) {
// The logic in here should be the inverse of the logic in BrowserRenderer's tryApplySpecialProperty.
// That is, we're doing the reverse mapping, starting from an HTML property and reconstructing which
// "special" attribute would have been mapped to that property.
if (elem instanceof HTMLInputElement) {
return (elem.type && elem.type.toLowerCase() === 'checkbox')
? { value: elem.checked }
: { value: elem.value };
}

if (elem instanceof HTMLSelectElement || elem instanceof HTMLTextAreaElement) {
return { value: elem.value };
}

return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public partial class BrowserEventDescriptor
public BrowserEventDescriptor() { }
public int BrowserRendererId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public string EventArgsType { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public Microsoft.AspNetCore.Components.Rendering.EventFieldInfo EventFieldInfo { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public int EventHandlerId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
}
}
Expand Down