Skip to content

Conversation

@Dave-Lowndes
Copy link

@Dave-Lowndes Dave-Lowndes commented Jan 23, 2024

Fixes #93189
Rework MSFormatDateTime to use the isDate parameter to try and reproduce the prior .NET FW version behaviour that used the Win32 GetDateFormat & GetTimeFormat APIs to only output date or time parts.

EDIT (@krwq): added Fixes ... syntax so that PR is linked with the issue

Rework MSFormatDateTime to use the isDate parameter to try and reproduce the prior .NET FW version behaviour that used the Win32 GetDateFormat & GetTimeFormat APIs to only output date or time parts.
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-System.Xml labels Jan 23, 2024
@ghost
Copy link

ghost commented Jan 23, 2024

Tagging subscribers to this area: @dotnet/area-system-xml
See info in area-owners.md if you want to be subscribed.

Issue Details

Rework MSFormatDateTime to use the isDate parameter to try and reproduce the prior .NET FW version behaviour that used the Win32 GetDateFormat & GetTimeFormat APIs to only output date or time parts.

Author: Dave-Lowndes
Assignees: -
Labels:

area-System.Xml, community-contribution

Milestone: -

@Dave-Lowndes
Copy link
Author

@dotnet-policy-service agree

@danmoseley
Copy link
Member

Thanks, can you add a test?

@Dave-Lowndes
Copy link
Author

Thanks, can you add a test?

I can't even build the project, so adding a test is beyond my (lack of) pay grade.

@tarekgh
Copy link
Member

tarekgh commented Jan 24, 2024

@krwq, could you kindly review and assess the proposed change?

@krwq

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@krwq

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@krwq
Copy link
Member

krwq commented Feb 20, 2024

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley danmoseley changed the title Proposed fix for issue #93189. Proposed fix for issue XSLT extension methods format-date & format-time Feb 20, 2024
@krwq

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@krwq

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@krwq

This comment was marked as off-topic.

@azure-pipelines

This comment was marked as off-topic.

@eiriktsarpalis eiriktsarpalis added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jul 9, 2024
Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

Per reviewer feedback, some basic regression testing is required before we merge this.

@Dave-Lowndes
Copy link
Author

Having already tested this change to my own satisfaction (via a stand-alone test project before I submitted the PR), if there's no one available and able to add unit tests you may as well abandon this pull request now.

@dotnet-policy-service dotnet-policy-service bot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Jul 23, 2024
@krwq
Copy link
Member

krwq commented Nov 18, 2024

@Dave-Lowndes if you share code you've used for testing I can turn that into unit-test and add to this PR on your behalf

@Dave-Lowndes
Copy link
Author

Dave-Lowndes commented Nov 18, 2024

@krwq There's not much to speak of. All I did was copy the proposed function change into a C# console project and fake out the internal XsdDateTime lines and make some test calls to it. You presumably won't need to replace XsdDateTime code.

    internal class Program
    {
// Copied from the internal code, you won't need this:
		private static CultureInfo GetCultureInfo(string lang)
		{
			Debug.Assert(lang != null);
			if (lang.Length == 0)
			{
				return CultureInfo.CurrentCulture;
			}
			else
			{
				try
				{
					return new CultureInfo(lang);
				}
				catch (System.ArgumentException)
				{
					//					throw new XslTransformException(SR.Xslt_InvalidLanguage, lang);
					throw;
				}
			}
		}

		public static string MSFormatDateTime(string dateTime, string format, string lang, bool isDate)
		{
			try
			{
				// XsdDateTime is private, so fake this out and use DateTime
#if true
				DateTime dt = DateTime.Parse(dateTime);
#else
				if (!XsdDateTime.TryParse(dateTime, XsdDateTimeFlags.AllXsd | XsdDateTimeFlags.XdrDateTime | XsdDateTimeFlags.XdrTimeNoTz, out XsdDateTime xdt))
				{
					return string.Empty;
				}

				DateTime dt = xdt.ToZulu();
#endif

				CultureInfo ci = string.IsNullOrEmpty(lang) ?
									CultureInfo.CurrentCulture :
									GetCultureInfo(lang);

				if (isDate)
				{
					DateOnly dateOnly = DateOnly.FromDateTime(dt);
					return dateOnly.ToString(!string.IsNullOrEmpty(format) ? format : null, ci);
				}
				else
				{
					TimeOnly timeOnly = TimeOnly.FromDateTime(dt);
					if (!string.IsNullOrEmpty(format))
					{
						return timeOnly.ToString(format, ci);
					}
					else
					{
						// Duplicate what the Win32 GetTimeFormat API returns
						// with an empty format - the long time string.
						return timeOnly.ToLongTimeString();
					}
				}
			}
			catch (Exception ex) when (ex is ArgumentException or FormatException)
			{
				return string.Empty;
			}
		}

        static void Main(string[] args)
        {
            string result;
            var dtStr = DateTime.Now.ToString();
            Console.WriteLine($"Result: {dtStr}");

            result = MSFormatDateTime( dtStr, string.Empty, CultureInfo.CurrentCulture.Name, true);
            Console.WriteLine($"Result: {result}");
            result = MSFormatDateTime( dtStr, string.Empty, CultureInfo.CurrentCulture.Name, false);
            Console.WriteLine($"Result: {result}");

            result = MSFormatDateTime(dtStr, "D", "", true);
            Console.WriteLine($"Result: {result}");
            result = MSFormatDateTime(dtStr, "T", "", false);
            Console.WriteLine($"Result: {result}");

            result = MSFormatDateTime(dtStr, "d", "", true);
            Console.WriteLine($"Result: {result}");
            result = MSFormatDateTime(dtStr, "t", "", false);
            Console.WriteLine($"Result: {result}");
        }
    }

@IDisposable
Copy link
Contributor

IDisposable commented Mar 13, 2025

I can build built a unit test for this specific bug... can you give me the simplest XML file you can repro with all the namespace declarations please (I'm not sure what ms: references)?

The template XSLT is:

<?xml version="1.0" encoding="UTF-8" ?>
<xsl:stylesheet version="1.0"
                xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
                xmlns:date="http://exslt.org/dates-and-times"
                xmlns:ms="urn:schemas-microsoft-com:xslt"
                exclude-result-prefixes="date">
    <xsl:output indent="yes" omit-xml-declaration="yes"/>
    <xsl:template match="data">
        <out>
            <dates>
                <test1>
                    <xsl:value-of select="ms:format-date(date, 'D')"/>
                </test1>
                <test2>
                    <xsl:value-of select="ms:format-date(date, 'd')"/>
                </test2>
                <test3>
                    <xsl:value-of select="ms:format-date(date, '')"/>
                </test3>
                <test4>
                    <xsl:value-of select="ms:format-date(date, 'd', 'en-US')"/>
                </test4>
                <test5>
                    <xsl:value-of select="ms:format-date(date, 'D', 'fr-FR')"/>
                </test5>
                <test6>
                    <xsl:value-of select="ms:format-date(date, 'd', 'fr-FR')"/>
                </test6>
            </dates>
            <times>
                <test1>
                    <xsl:value-of select="ms:format-time(date, 'T')"/>
                </test1>
                <test2>
                    <xsl:value-of select="ms:format-time(date, 't')"/>
                </test2>
                <test3>
                    <xsl:value-of select="ms:format-time(date, '')"/>
                </test3>
                <test4>
                    <xsl:value-of select="ms:format-date(date, 't', 'en-US')"/>
                </test4>
                <test5>
                    <xsl:value-of select="ms:format-date(date, 'T', 'fr-FR')"/>
                </test5>
                <test6>
                    <xsl:value-of select="ms:format-date(date, 't', 'fr-FR')"/>
                </test6>
            </times>
        </out>
    </xsl:template>
</xsl:stylesheet>

@IDisposable
Copy link
Contributor

IDisposable commented Mar 14, 2025

I added unit-tests that currently PASS in PR #113507

@IDisposable
Copy link
Contributor

IDisposable commented Mar 24, 2025

Now that the unit test for this have landed in PR #113507 (and are coded for the old and wrong return values), we need to update the test data in this PR before merging or the new unit tests will fail

We need to include an update to the expected values in bug93189.xml test data changing:

out/dates/test3 to be 06/15/2009
out/times/test3 to be 1:45 PM // important, that's not a space, that's a U-202F NARROW NO-BREAK SPACE

@tarekgh tarekgh modified the milestones: 9.0.0, 10.0.0 Mar 24, 2025
@Dave-Lowndes Dave-Lowndes closed this by deleting the head repository Apr 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Xml community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XSLT extension methods format-date & format-time both output date & time when using the empty string (default) format

6 participants