fix(core): add hydration protected elements

[arturovt]

No description provided.

[AndrewKushnir]

@arturovt as we've discussed offline, the proposed fix would not work due to potential performance issues. This extra logic should be limited to the following scenario:

• when an element is an iframe and it was hydrated
• and when we try to set the src attribute for the first time

There are 2 possible cases:

1. Static src attribute (e.g. <iframe src="...">).
2. When src attribute is set as a binding (e.g. <iframe [src]="...">).

Those 2 cases are being handled by different code paths (static case, binding case). Static attributes are applied when an element is created/hydrated and we can handle that. The binding case is more complicated: it's invoked as a part of change detection and currently there is no information whether an underlying element was created or hydrated, so we need to store more info about it (but only for iframes with src attributes).

Here is a proposed solution:

1. Add a new field to the DehydratedView interface (code) to store an initial value of the src, which it currently has in the DOM. The field might have the following shape: Map<number, string>, where number would represent a TNode index and a value is the current value of the src attribute.
2. In hydration code (here), once we find an element in the DOM, we can check whether an element is an iframe and if so, read the value of the src attribute and store it in the lView[HYDRATION] field (we've updated the type for it in Step 1). Note: the field should be null by default, so we don't create empty Maps for relatively rare cases when we need it, we can create it lazily when we come across an iframe.
3. Update the code path for static attributes (in this function) and instead of always doing renderer.setAttribute(native, attrName, attrVal as string), we can invoke a different function (for example, _setAttribute), which would do the regular call by default, but we replace its implementation if hydration is enabled using this approach. The hydration-aware implementation may look like this:

function _setAttributeWithHydrationSupport(lView: LView, tNode: TNode, tagName: string, element: RElement, name: string, value: string|TrustedHTML|TrustedScript|TrustedScriptURL, namespace?: string|null) { 
  if (
      // Are we in the process of creating/updating a view the first time?
      (lView[FLAGS] & LViewFlags.FirstLViewPass) &&
      // Do we have extra hydration info?
      (typeof lView?.[HYDRATION]?.protectedAttrs !== 'undefined') &&
      // Is it a "protected" attribute?
      isHydrationProtectedAttr(tagName, name) && 
      // Do we have a "protected" attribute on this node and whether the value is the same?
      lView[HYDRATION].protectedAttrs[tNode.index] === value) {

    // If answers to the questions above are all "yes" - exit, we do not want to reset the value,
    // since it may negatively affect performance and UX (e.g. in the `<iframe src="...">` case,
    // this would cause a reload within the iframe).
    return;
  }
  const renderer = lView[RENDERER];
  renderer.setAttribute(element, name, value, namespace);
}

The isHydrationProtectedAttr may look like this:

function isHydrationProtectedAttr(tagName: string, attrName: string): boolean {
  // Note: this list can be expanded if there are more cases.
  return tagName === 'iframe' && attrName === 'src';
}

4. For the binding case, we can now use the _setAttribute, which will use hydration-aware implementation and prevent overriding src for the first update pass.
5. Put together a few tests (in this spec) to verify that the src is not overridden during hydration.

Additional note: there might be other elements affected by the same problem, we'd like to ask if you could check the spec for elements like <audio>, <object>, <picture> and others that refer to remote resources, to see if any of them might be affected and should be included into the fix.

Please let me know if any additional information is required.

[arturovt]

Here's the list of valid and invalid cases: elements marked in red reload resources when hydrated. We don't consider cases when the cache is disabled through devtools or the remote resource returns a 302 status code.

• ✅

<img src="/assets/img_girl.jpg" />

• ✅

<picture>
  <source media="(min-width: 650px)" srcset="/assets/img_food.jpg" />
  <source media="(min-width: 465px)" srcset="/assets/img_car.jpg" />
  <img src="/assets/img_girl.jpg"  />
</picture>

• ✅

<svg>
  <image xlink:href="/assets/angular.svg" />
</svg>

• ✅

<video controls autoplay>
  <source src="/assets/mov_bbb.mp4" type="video/mp4" />
  <source src="/assets/mov_bbb.ogg" type="video/ogg" />
</video>

• ✅

<audio controls src="/assets/t-rex-roar.mp3"></audio>

• ✅

<video controls src="/assets/friday.mp4">
  <track default kind="captions" srclang="en" src="/assets/friday.vtt" />
</video>

---

• ❌

<object
  type="application/pdf"
  data="/assets/In-CC0.pdf"
></object>

• ❌

<embed type="video/webm" src="/assets/flower.mp4" />

• ❌

<iframe src="https://www.youtube-nocookie.com/embed/7HVMn1TpXKQ"></iframe>

[AndrewKushnir]

@arturovt thanks for additional information, this is very helpful. We can extend the isHydrationProtectedAttr function described in my previous comment to support <object> and <embed> cases as well. Please let us know if you might be interested in updating this PR as described in #52478 (comment) and also let us know if you have any questions about the proposed fix.

[arturovt]

@AndrewKushnir I am interested in providing the fix; I'll need to go through your comments and start implementing it locally. I'll message you if I have any questions.

[rhutchison]

Thanks @arturovt and @AndrewKushnir for your efforts on this.

[AndrewKushnir]

@arturovt thanks for implementing this 👍 I've added this PR to the v18 milestone.

If you get a chance, could you please try to find corresponding sections of HTML spec that talk about this specific behavior of the <iframe>, <object> and <embed> elements, so that we can include it as comments into the code for future reference?

[arturovt]

@AndrewKushnir I went through the HTML standard and I may guess it doesn't explicitly mandate this behavior or provide detailed instructions on how user agents should handle this aspect of network requests. Because if we talk about img.src - setting the src attribute to the same value again on the img element doesn't trigger resource reload and it's a common behavior implemented by all user agents. Considering the iframe.src case, I may guess that the content of an iframe can be dynamic and it may contain interactive elements or scripts that require reloading when the source URL changes (took this statement from Google search).

I am confident that what we're doing in this PR doesn't go against the spec because we're essentially targeting that hydration case with the very first render. (Please correct me if I'm wrong...).

Out of curiosity, wanna check what engines do internally...

UPD: I examined the behavior of Safari's JSC for images and iframes.

Each HTML element class has a hook called attributeChanged.

For iframes, HTMLIFrameElement::attributeChanged triggers HTMLFrameElementBase::attributeChanged, which then determines the attribute being changed. If name == srcAttr, it invokes setLocation, which in turn calls openURL.

In the case of images, it's more intricate and aligns with our previous discussion. HTMLImageElement::attributeChanged employs a switch-case structure, and if it's case AttributeNames::srcAttr, it performs this check:

if (oldValue != newValue)
  selectImageSource(RelevantMutation::Yes);
else
  m_imageLoader->updateFromElementIgnoringPreviousErrorToSameValue();

And updateFromElementIgnoringPreviousErrorToSameValue checks whether image->allowsCaching() and does nothing if it has caching or does not have running service worker.

But you know, it's more like interesting information to keep in mind. Unfortunately, such implementation details are not included in the specification.

[AndrewKushnir]

@arturovt thanks for the PR, the code looks great! I've just left a few comments with some proposals.

Could you please also mark this PR as "Ready for review" via GitHub UI?

[arturovt]

Could you please also mark this PR as "Ready for review" via GitHub UI?

Done.

Symbol tests are failing, I will update symbols later. Gotta play with unit tests first to spy on the setAttribute.

[arturovt]

@AndrewKushnir I have addressed your comments and also updated unit tests with spying on the setAttribute of the renderer.

[AndrewKushnir]

@arturovt thanks for addressing the feedback! I've left a comment about some extra testing cases.

[AndrewKushnir]

Could you please also add some logic here to change the value and make sure that the setAttribite was invoked as needed (i.e. that the protection logic doesn't prevent subsequent value updates)?

[arturovt]

@AndrewKushnir your last comment helped me realize I was moving in the wrong direction. We only covered the attribute case (<iframe [attr.src]="..."), but <iframe [src]=value actually calls elementPropertyInternal and then the renderer's setProperty method.

I have updated packages/platform-server/test/hydration_spec.ts with a value update to ensure that setAttribute is called after hydration is completed. You may notice that I updated bindings to attribute bindings ([attr.src]).

Wondering whether we have to update the setProperty implementation too?