empira / PDFsharp

PDFsharp and MigraDoc Foundation for .NET 6 and .NET Framework
https://docs.pdfsharp.net/
Other
492 stars 114 forks source link

Initializing Migradoc DefaultPageSetup throws when CurrentCulture is a neutral culture #97

Closed DanSmith closed 6 months ago

DanSmith commented 6 months ago

Reporting an Issue Here

Expected Behavior

DefaultPageSetup should initialize without throwing an exception

Actual Behavior

When the CurrentCulture is a neutral culture, the RegionInfo constructor will throw an argument exception. From the Microsoft RegionInfo constructor docs: This constructor throws an ArgumentException if name is a neutral culture (such as en for English).

https://github.com/empira/PDFsharp/blob/b28bc3265e0a15302c04b7b91da0d915a591e167/src/foundation/src/MigraDoc/src/MigraDoc.DocumentObjectModel/DocumentObjectModel/PageSetup.cs#L393

Steps to Reproduce the Behavior

Issue.zip in src\MigraDoc\src\IssueSubmission

CultureInfo.CurrentCulture = new CultureInfo("en"); // A neutral culture
var doc = new Document();
var initialize = doc.DefaultPageSetup; // RegionInfo constructor throws

System.ArgumentException: 'The region name en should not correspond to neutral culture; a specific culture name is required. Parameter name: name'

Suggested possible fixes

Test if the current culture is a neutral region, and if it is, use the invariant culture

            var culture = CultureInfo.CurrentCulture;
            if (culture.IsNeutralCulture)
            {
                culture = CultureInfo.InvariantCulture;
            }
            var regionInfo = culture.Name.Length > 0 ? new RegionInfo(culture.Name) : null;
            var isMetric = regionInfo?.IsMetric ?? true;

or just use the thread's CurrentRegion as oppose to looking through the CurrentCulture

            var regionInfo = RegionInfo.CurrentRegion;
            var isMetric = regionInfo?.IsMetric ?? true;
ThomasHoevel commented 6 months ago

Thanks for the feedback. This issue won't be addressed with 6.1.0 Preview 2 coming soon, but should be resolved with Preview 3.

StLange commented 6 months ago

Thanks Dan. Well, we first check whether the system of measurement is metric or not and then set the page size to A4 anyway. Makes less sense. I removed the superfluous code. It gets published in 6.1 Preview 2 on Thursday.