Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the web.config errors for the new Localization module #4267

Closed
wants to merge 1 commit into from

Conversation

donker
Copy link
Contributor

@donker donker commented Nov 1, 2020

Unfortunately the XmlMerge in DNN does not allow a collision check when doing any other action than "update". This prompted two Localization modules to be present in the web.config for new installs which makes the site unuseable. This PR changes the action to be "update". Note that it means we don't control where in the modules pipeline the module sits. But this has become a moot point since we changed the event order for this module.

In the future we should adjust the XmlMerge class to cater for collision control for the other actions, IMHO.

@donker donker added this to the 9.8.0 milestone Nov 1, 2020
@donker donker requested a review from bdukes November 1, 2020 11:02
@bdukes
Copy link
Contributor

bdukes commented Nov 1, 2020

#4264 removed the localization module from the config to start, so there shouldn't be duplicates now.

@bdukes
Copy link
Contributor

bdukes commented Nov 1, 2020

And I think we still want it to run as early as possible.

@mitchelsellers
Copy link
Contributor

I agree with @bdukes here

@valadas
Copy link
Contributor

valadas commented Nov 1, 2020

Yeah I thought it was fixed, at least I don't have duplicates on a clean install from a build on the release branch... @donker was this tested after Brian's PR got merged ?

@donker
Copy link
Contributor Author

donker commented Nov 1, 2020

When I tested, Brian's PR hadn't been merged yet. Now it should be fine.

@donker donker closed this Nov 1, 2020
@donker donker deleted the fixwebconfig branch July 24, 2022 08:28
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.

4 participants