Skip to content

Fixed an issue with wrong localization detection#6505

Merged
mitchelsellers merged 1 commit intodnnsoftware:developfrom
valadas:missing-loc-provider
Apr 14, 2025
Merged

Fixed an issue with wrong localization detection#6505
mitchelsellers merged 1 commit intodnnsoftware:developfrom
valadas:missing-loc-provider

Conversation

@valadas
Copy link
Copy Markdown
Contributor

@valadas valadas commented Apr 14, 2025

I did a git bisect which pointed me to the commits in #6038 to rollup the sql scripts for v10. This made no sense to me since it's not really related to web.config and @uzmannazari pointed out in #6503 that a similar issue was caused by the LocalizationModule missing from the web.config. Also I could not reproduce the issue on upgrades. Then I thought since we have that v10 rollup, it uses the 10.00.00.config for the web.config xml merge scripts and up. Investigating the commit history of that file and comparing with 9.8.0 one I found that in #4251, a bugfix was done to ensure that the LocalizationModule is right after the UrlRewriteModule.

This means that upon upgrades it would be in the right place but with clean installs, that would never run and it was also missing from the base web.config, thus making it not be there at all.

I was debating between putting this fix in the 10.00.00.config or just in the web.config I thought the web.config is fine as upon upgrades, the order would already have been fixed (as long as we document to upgrade to latest v9 before upgrading to v10). For clean installs it would just already be at the right place to start with.

Closes #6503

I did a `git bisect` which pointed me to the commits in dnnsoftware#6038 to rollup the sql scripts for v10. This made no sense to me since it's not really related to web.config and @uzmannazari pointed out in dnnsoftware#6503 that a similar issue was caused by the LocalizationModule missing from the web.config. Also I could not reproduce the issue on upgrades. Then I thought since we have that v10 rollup, it uses the 10.00.00.config for the web.config xml merge scripts and up. Investigating the commit history of that file and comparing with 9.8.0 one I found that in dnnsoftware#4251, a bugfix was done to insure that the LocalizationModule is right after the UrlRewriteModule.

This means tha upon upgrades it would be in the right place but with clean installs, that would never run and it was also missing from the base web.config, thus making it not be there at all.

I was debating between putting this fix in the 10.00.00.config or just in the web.config I thought the web.config is fine as upon upgrades, the order would already have been fixed (as long as we document to upgrade to latest v9 before upgrading to v10). For clean installs it would just already be at the right place to start with.
@valadas valadas added this to the 10.0.1 milestone Apr 14, 2025
@valadas valadas mentioned this pull request Apr 14, 2025
2 tasks
Copy link
Copy Markdown
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent @valadas - thank you so much! 🎉

@uzmannazari
Copy link
Copy Markdown
Contributor

@valadas
investigation and after logging the entire installation process, we discovered that even other critical nodes from the 10.00.00.config file are not being merged into the final web.config during a clean install.

For instance, the ServiceRequestScopeModule, which is clearly defined in the config merge file, never gets added to the output web.config. This indicates a broader problem—it seems the installer is unable to properly read or apply the XML config merge files during installation.

We confirmed this by adding detailed logging to the XmlMerge.cs logic and saw that neither the expected InsertNode actions were triggered, nor were the relevant configuration files even being processed as expected.

This goes beyond a simple placement or syntax issue; it looks like the config merge engine is either silently skipping or failing to locate and process valid .config files, which can lead to a broken runtime environment after a clean install.

If needed, I can provide the logs and the exact config files we used for testing.

@valadas
Copy link
Copy Markdown
Contributor Author

valadas commented Apr 14, 2025

@uzmannazari absolutely, the more information the better.

@uzmannazari
Copy link
Copy Markdown
Contributor

Thanks! I’ve prepared the full logs and the related config files we used during testing.

They clearly show that the 10.00.00.config was reached during installation, but certain actions like insertbefore or insertafter were never executed—even though other parts of the same config file were successfully applied.

I'll upload everything shortly for your review. Let me know if you'd prefer a GitHub gist, direct attachment here, or a different way to share them.

XmlMerge.zip

Also, if there's anything specific you'd like us to trace further in the XmlMerge process, I'm happy to run

Copy link
Copy Markdown
Contributor

@mitchelsellers mitchelsellers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mitchelsellers mitchelsellers merged commit 17e3523 into dnnsoftware:develop Apr 14, 2025
3 checks passed
@mitchelsellers
Copy link
Copy Markdown
Contributor

I've merged this one @valadas @uzmannazari maybe we create a separate issue to document other misses?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Page Language Attribute

4 participants