VariationContext not set correctly after upgrading from 13.3.0

[jemayn]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

13.3.0 - 13.3.2

Bug summary

After upgrading to 13.3.1 and then 13.3.2 we've noticed on a site that the node we get in the FindContent method of an IVirtualPageController no longer gets the correct culture context.

Specifics

Happens from 13.3.1 and up on virtually routed pages, it works fine on regular Umbraco node routes.

I've uploaded a test repo that I've used to reproduce here if that is helpful: https://github.com/jemayn/culture-bug-umbraco

Steps to reproduce

If I have a structure like this in the backoffice on two languages where the names are different:

VirtualController:

using BugTesting.Models;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Common.PublishedModels;

namespace BugTesting.Controllers;

public class VirtualProductController : UmbracoPageController, IVirtualPageController
{
    private readonly IUmbracoContextAccessor _umbracoContextAccessor;
    private readonly IPublishedValueFallback _publishedValueFallback;

    public VirtualProductController(
        ILogger<UmbracoPageController> logger,
        ICompositeViewEngine compositeViewEngine,
        IUmbracoContextAccessor umbracoContextAccessor,
        IPublishedValueFallback publishedValueFallback) : base(logger,
        compositeViewEngine)
    {
        _umbracoContextAccessor = umbracoContextAccessor;
        _publishedValueFallback = publishedValueFallback;
    }

    public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext)
    {
        if (!_umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext))
        {
            return null;
        }

        var rootContent = umbracoContext.Content?.GetAtRoot();

        var productPlaceholderPage = rootContent?.FirstOrDefault()?.Children(x => x.ContentType.Alias == ProductPlaceholder.ModelTypeAlias)?.FirstOrDefault();

        if (productPlaceholderPage is null)
        {
            return null;
        }

        return new ProductViewModel(productPlaceholderPage, _publishedValueFallback);
    }

    [Route("p/{sku}")]
    [Route("{lang}/p/{sku}")]
    public async Task<IActionResult> Index([FromRoute] string sku)
    {
        if (CurrentPage is not ProductViewModel productViewModel)
        {
            return NotFound();
        }

        productViewModel.Sku = sku;

        return View("~/Views/ProductPlaceholder.cshtml", productViewModel);
    }
}

Viewmodel:

using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Web.Common.PublishedModels;

namespace BugTesting.Models;

public class ProductViewModel : ProductPlaceholder
{
    public ProductViewModel(IPublishedContent content, IPublishedValueFallback publishedValueFallback) : base(content, publishedValueFallback)
    {
    }

    public string? Sku { get; set; }
}

View:

@model BugTesting.Models.ProductViewModel
@{
	Layout = "Master.cshtml";
}

<h1>Product sku: @Model.Sku</h1>

<p>Culture: @Model.GetCultureFromDomains()</p>
<p>Pagename: @Model.Name</p>

Then on 13.3.0 I get this result:

And on 13.3.2 I get:

So while it consistently can get the correct culture from the GetCultureFromDomains() method, the variationContext is wrong and it retrieves the english value for name and other properties.

It works if I add
VariationContextAccessor.VariationContext = new VariationContext(Model.GetCultureFromDomains()); at the top of the view, but not too keen on adding that in multiple views 🙂

Expected result / actual result

I'd expect it to set the culture of the content to the current culture as it did before the change.

[github-actions]

Hi there @jemayn!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[nikolajlauridsen]

Hey @jemayn, thanks a lot for the detailed description, but I'm having some issues running your test site, I'm getting some uSync error.

However, this sounds like the same issue as #16555 which should have been closed by #16572, which is actually out in RC2 now.

I'll go ahead and close this for now, however please do feel free to comment and I'll re-open it if this turns out to not be the case 😄

[jemayn]

@nikolajlauridsen that's great - I had a look for similar issues but I must have missed yours in the sea of v14 ones 😅

I tested on my project with 13.4.0 RC2 and it fixed the problem indeed, so guess we will just wait for that one 🙂

[nikolajlauridsen]

That's great, glad to hear it, fortunately, 13.4 is going out tomorrow, so you won't have to wait for long 😄

[kasparboelkjeldsen]

From someone who was just bitten hard by this, here is what I'm dying to know, and isn't obvious for me from the above - we only experience this in Edge and Brave and not in Chrome. How can localization hit differently on two browsers both in incognito on something I'd think would be a 100% serverside operation ?

-oh no, it's -not- fixed! Still a difference on edge and chrome on 13.4.0-rc2 !