Messing up your page layout with a fun Razor mistake

Sometimes it's the simplest things that can trip up development work. A case in point is a bug ticket I got handed recently, which I think shows the need to understand your tools when writing code. Read on to avoid an interesting mistake with code flow in Razor views...

The issue url copied!

QA raised a bug on a UI component that showed some images for a product details page. The design had the page broken up into regions, so it started with two columns - images on left and descriptive text on the right. Below this was a full-width region with some other bits in it. Because that right column of text was potentially long, the images were supposed to be "sticky" as you scrolled down the page, until you reached the end of the two column part of the page. They were supposed to stop there, before overlapping the contents of the single-column section below.

But QA reported a page where scrolling down would cause the images to start overlapping this next content section - in fact the images would carry on being sticky all the way down the page to the footer if you kept scrolling...

The cause url copied!

After looking into which instances of this page worked, and which ones did not, the issue appeared related to whether the product in question had any reviews or not. But reviews were being rendered by a different component to the one which was reported as broken. Digging through the source for reviews showed some code along the lines of:

<section>
  @if(Model.HasReviews) {
    foreach(var review in Model.Reviews) {
      @RenderReview(review)
    }
    <div><a href="https://reviewsite.com/product/wxyz">Link to more reviews</a></div>
  } else {
    return;
  }
</section>

					

It's a slightly contrived example to highlight the issue, but what it was supposed to do was only render the reviews and the link if there were reviews to show. But in the scenario with no reviews, "view source" in the browser showed this component emitted a <section> starting tag but never provided its closing </section> tag.

When this invalid HTML arrived at the browser it dutifully tried to parse and "fix" the unbalanced markup it received. But whatever rules it followed caused it to move the floating <img/> element to be a child of an element one level higher up the tree than the CSS expected - which could be seen in the DevTools inspector. And this meant the image could float further than expected. (On closer inspection, it also broke some other stuff on the page too - but those issues hadn't been raised in the QA ticket)

That behaviour (where the browser is trying very hard to fix the mess) caused some confusion. When you look at the results using the Developer Tools you're seeing the "fixed" DOM. It's structure can be different to what "View Source" would show you. And that difference can cover up a bug like this if you're not expecting this behavour.

What's happening here url copied!

The perils of not making your HTML valid is a separate issue. I'm perhaps showing my "grew up int the age of XML and XSLT being the new hotness" age here, but I much prefer using XHTML to the "some elements are self-closing" approach of standard HTML. I find it easier to spot issues in that model.

But the key take-away for me from this issue is to understand how Razor files get turned into code, and hence how flow-control operations like return will affect their execution.

When your app renders a view, it's compiled into a method which intermixes lines that write literal strings to the output stream with any logic that the Razor file includes. If you decompile the DLL generated from the Razor above, it looks something like:

public class _Page_Views_Example_cshtml : WebViewPage<ExampleModel>
{
    public override void Execute()
    {
        this.WriteLiteral("<section>");
        if (Model.HasReviews)
        {
            foreach(var review in Model.Reviews)
            {
                // Render the review
            }
            this.WriteLiteral("<div><a href=\"https://reviewsite.com/product/wxyz\">Link to more reviews</a></div>");
        }
        else
        {
            return;
        }
        this.WriteLiteral("</section>");
    }
}

					

And that should highlight why this issue happened: Because it's a single function, that return causes the whole thing to exit and the unbalanaced markup to be generated. If there are no reviews, the code can never get to outputting that </section>.

So think carefully about how Razor translates into C# when you're implementing logic. Is using return for flow control here really the right approach? I'd argue the vast majority of the time it isn't. In this scenario the whole else clause didn't need to be there. And removing it was the fix for this bug.

↑ Back to top

• Feel like sharing?
• ⇒ On BlueSky
• ⇒ On LinkedIn
• ⇒ On Mastodon
• ⇒ On Email