aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

PageTitle Headoutlet + Changing layouts causes rendering to fail

Open vflame opened this issue 2 years ago • 7 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

When the _Host.cshtml has a html <title> defined in the <head> and dynamic layouts are used with the <PageTitle> component, rendering fails when the layout is changed. Maybe related to this issue? https://github.com/dotnet/aspnetcore/issues/42902

Expected Behavior

HeadOutlet should handle title updates properly when switching layouts. If <head> isn't permitted to have a

then a more meaningful error/exception should be provided. This took a long time to debug. <h3>Steps To Reproduce</h3> <p>Reproduction: https://github.com/vflame/BlazorPageTitleLayoutError</p> <p><img src="https://user-images.githubusercontent.com/12207061/209449489-dde691c8-6df4-410d-b4a4-fa6a2c8b014d.gif" alt="pagetitleerror"></p> <h3>Exceptions (if any)</h3> <p><em>No response</em></p> <h3>.NET Version</h3> <p>.net 7</p> <h3>Anything else?</h3> <p><em>No response</em></p>

vflame avatar Dec 24 '22 20:12 vflame

Another thing I wanted to add--any usage of <PageTitle> with dynamic layouts is very flaky and causes desyncs during DOM updates based on timing/rendering speed/user iteractions: One or more errors occurred. (TypeError: Cannot read properties of null (reading 'insertBefore')) ---> System.InvalidOperationException: TypeError: Cannot read properties of null (reading 'insertBefore') at Microsoft.AspNetCore.Components.RenderTree.Renderer.InvokeRenderCompletedCallsAfterUpdateDisplayTask(Task updateDisplayTask, Int32[] updatedComponents)

vflame avatar Dec 26 '22 19:12 vflame

Thanks for contacting us, @vflame. @MackinnonBuck can you please investigate this when you're back? Thanks!

mkArtakMSFT avatar Dec 27 '22 17:12 mkArtakMSFT

Hi @vflame,

I tried you repro and I wasn't able to reproduce the bug:

https://user-images.githubusercontent.com/10456961/210618275-02441293-8f97-4a54-bbfb-d8b8670febf7.mp4

Could you delete the bin and obj folders, clear browser cache, and run using dotnet run to see if you still experience the issue? Thanks.

MackinnonBuck avatar Jan 04 '23 17:01 MackinnonBuck

Hi @vflame. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost avatar Jan 04 '23 17:01 ghost

Hi @vflame,

I tried you repro and I wasn't able to reproduce the bug:

repro_works.mp4 Could you delete the bin and obj folders, clear browser cache, and run using dotnet run to see if you still experience the issue? Thanks.

I cleared the bin/obj and ran with dotnet and see the following in the console output below.

I did some more testing and found that it errors out on Chrome (Version 108.0.5359.125 (Official Build) (64-bit)), but is fine on Firefox (Version 108.0.1 (64-bit)).

Out of curiosity, I re-tested the issue I opened here: https://github.com/dotnet/aspnetcore/issues/40056 and found that it's only impacted by Chrome as well (works fine in Firefox).

pagetitleerror2

warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100]
      Unhandled exception rendering component: TypeError: Cannot read properties of null (reading 'insertBefore')
      System.InvalidOperationException: TypeError: Cannot read properties of null (reading 'insertBefore')
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.InvokeRenderCompletedCallsAfterUpdateDisplayTask(Task updateDisplayTask, Int32[] updatedComponents)
fail: Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost[111]
      Unhandled exception in circuit '9y2yAhQ18o55BX0Hy0oexn-PxyrKK_oTLYUW4-Hz1QI'.
      System.AggregateException: One or more errors occurred. (TypeError: Cannot read properties of null (reading 'insertBefore'))
       ---> System.InvalidOperationException: TypeError: Cannot read properties of null (reading 'insertBefore')
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.InvokeRenderCompletedCallsAfterUpdateDisplayTask(Task updateDisplayTask, Int32[] updatedComponents)
         --- End of inner exception stack trace ---

vflame avatar Jan 04 '23 22:01 vflame

@vflame Thanks, that helps. Did clearing browser cache make a difference at all? I also see that there are some extensions active in Chrome - does disabling those make a difference? What happens in Edge?

Thanks!

MackinnonBuck avatar Jan 04 '23 23:01 MackinnonBuck

I was able to narrow it down to a specific extension that I had -- it's one that transforms the page title and appends the url in the title with the following source:

'use strict';

(function() {

    // Ignore the page if <head> does not exist or <head urlintitle="disabled">.
    if (!document.head || document.head.getAttribute('urlintitle') == 'disabled') {
        return;
    }

    // The last known title set by the page, before formatting.
    // Reset to null after each update.
    let last_original_title = null;

    // The last formatted title before any browser post-processing.
    let last_formatted_title = null;

    // The last formatted_title_suffix returned by format_title_update.
    let last_formatted_title_suffix = null;

    // The last formatted title after browser post-processing (e.g. trimming).
    let last_postprocessed_title = null;


    function requestUpdateTitle() {
        const current_title = document.title;

        // Drop redundant update requests:
        // - an update is already pending for the current title
        // - the current title is the result if the last update
        if (last_original_title === current_title ||
            last_postprocessed_title === current_title) {
            return;
        }

        last_original_title = current_title;
        last_formatted_title = null;

        chrome.extension.sendRequest({
            name: 'get_constants'
        }, updateTitle);
    }

    function updateTitle(constants) {
        // Explicitly copy the required location fields that Chrome strips out.
        const location_copy = {};
        Object.keys(constants.LOCATION_FIELDS)
            .forEach(
                field => location_copy[field] = document.location[field]);

        // Ask the background script to format the title.
        chrome.extension.sendRequest({
                name: 'format_title_update',
                location: location_copy,
                filtering_url: document.location.href,
                title: last_original_title,
                previous_formatted_title_suffix: last_formatted_title_suffix,
            },
            result => {
                if (result) {
                    setFormattedTitle(
                        result.formatted_title,
                        result.formatted_title_suffix);
                }
            });
        last_original_title = null;
    }

    function setFormattedTitle(formatted_title, formatted_title_suffix) {
        // Set the title only if it has changed, to avoid recursive notifications.
        if (last_formatted_title !== formatted_title) {
            last_formatted_title = formatted_title;
            document.title = formatted_title;
            last_postprocessed_title = document.title;
            last_formatted_title_suffix = formatted_title_suffix;
        }
    }

    // Calls requestUpdateTitle() on future, programmatic title changes.
    // Listens for all <head> updates, not just <title> because the page can delete
    // it with document.querySelector('head > title').remove().
    // Known limitation: do not handle document.head.remove() by listening for
    // all of document, because deleting <head> is unlikely and breaks the
    // connection between document.title and the tab title.
    function registerTitleMutationObserver() {
        const observer = new window.MutationObserver(
            (mutations, observer) => requestUpdateTitle());
        observer.observe(
            document.head, {
                subtree: true,
                childList: true,
            });
    }

    requestUpdateTitle();
    registerTitleMutationObserver();

})();

This is causing the issues I'm seeing since the page works once I've disabled that exact extension and makes sense given how Blazor rendering + DOM works.

It's not unreasonable for users to be running extensions that could mutate the DOM unexpectedly. Is there a recommended way to catch/handle this type of exception, possibly without aborting the circuit? Is <div id="blazor-error-ui"> the best choice? These type of errors seems to escape the ErrorBoundary component as well (since they're occurring on the JS side of things).

As an enhancement, is it possible to return to Blazor what the offending DOM update/element was?

vflame avatar Jan 05 '23 03:01 vflame

Thanks for confirming the cause of the issue, @vflame.

As an enhancement, is it possible to return to Blazor what the offending DOM update/element was?

It might be possible for Blazor to somehow provide some info about the location in the DOM tree where the rendering error occurred. However, catching invalid DOM mutations proactively could end up being a nontrivial task, because the distinction needs to be made between mutations made by Blazor vs. external, "invalid" DOM mutations.

This issue has been moved to the "Discussions" milestone so we can gather more feedback from the community and measure the impact of this issue.

Thanks again!

MackinnonBuck avatar Jan 05 '23 18:01 MackinnonBuck