Fix pager HTML validation error: remove invalid <a> when disabled

[ziadalazwak]

When rendering the Pager shape, the “Previous” link is shown as disabled on the first page.
However, it was still outputting an element with a rel attribute but no href, which causes an HTML validation error.

Example before fix:

<li class="pager__item page-item disabled">
  <a rel="prev"><</a>
</li>

This PR updates the ActionLink shape to remove the entirely when disabled, while keeping the <li class="disabled"> placeholder for consistent layout.

Example after fix:

<li class="pager__item page-item disabled"></li>

Fixes #17907

[github-actions]

Thank you for submitting your first pull request, awesome! 🚀 If you haven't already, please take a moment to review our contribution guide. This guide provides helpful information to ensure your contribution aligns with our standards. A core team member will review your pull request.

If you like Orchard Core, please star our repo and join our community channels.

[ziadalazwak]

@dotnet-policy-service agree

[hishamco]

Please don't add many whitespaces next time

[ziadalazwak]

Applied your suggestion, thanks @hishamco

[hishamco]

Why was this removed?

[ziadalazwak]

I removed shape.Attributes.Remove("href") because when a pager item is disabled,
we now skip rendering the <a> tag entirely. Since there’s no <a> in that case,
there’s no need to remove href anymore.

[ziadalazwak]

Hi @hishamco, just wanted to follow up on your last comment. Since the tag is no longer rendered for disabled items, I removed the shape.Attributes.Remove("href") call as it’s not needed anymore. Could you please confirm if this resolves your concern?

[hishamco]

Could you please take a GIF screenshot to see what it looks like now

[ziadalazwak]

As shown in the GIF below, the pager now behaves correctly:

• On the first page, the “Previous” and “First” links are disabled without rendering invalid <a> tags.

[hishamco]

@Piedone, do you have anything to add here, or shall we merge

[Piedone]

I didn't check the code (on mobile) but from the above comments the results look good.

[hishamco]

@wAsnk, do you want to check this PR?

[ziadalazwak]

Hi @hishamco, @Piedone — just wanted to follow up to see if there’s anything else needed from my side to move this PR forward. Thanks again for the review and guidance!

[Piedone]

Nothing else from my side, up to Hisham.

[wAsnk]

@wAsnk, do you want to check this PR?

Looks good to me.

[Piedone]

@hishamco can't this be merged?

[ziadalazwak]

Hi @hishamco,

It’s been a while since the last update, and I noticed the PR has already passed all checks and received approvals. Could you please let me know if there’s any reason it hasn’t been merged yet?

I’ve been waiting for some feedback or final confirmation, and it’s a bit discouraging not to hear back after addressing all comments and providing the requested changes. I’d really appreciate an update on this.

Thanks,
Ziad

[gvkries]

Removing the entire <a> tag also removes its inner content, including text and icons, because the associated value is no longer rendered. This can unintentionally break existing themes and layouts.

For example, in the admin interface, the pager changes from:

to:

To preserve the structure and visual integrity, only the rel attribute should be removed not the entire anchor tag.