Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,6 @@ git commit -m "Fix: Description of the change"
- `.github/instructions/android.instructions.md` - Android handler implementation
- `.github/instructions/xaml-unittests.instructions.md` - XAML unit test guidelines

### Opening PRs

All PRs are required to have this at the top of the description:

```
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!
```

Always put that at the top, without the block quotes. Without it, users will NOT be able to try the PR and your work will have been in vain!



## Custom Agents and Skills

Expand Down
15 changes: 2 additions & 13 deletions .github/skills/pr-finalize/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,10 @@ Examples:
## Description Requirements

PR description should:
1. Start with the required NOTE block (so users can test PR artifacts)
2. Include the base sections from `.github/PULL_REQUEST_TEMPLATE.md` ("Description of Change" and "Issues Fixed"). The skill adds additional structured fields (Root cause, Fix, Key insight, etc.) as recommended enhancements for better agent context.
3. Match the actual implementation
1. Include the base sections from `.github/PULL_REQUEST_TEMPLATE.md` ("Description of Change" and "Issues Fixed"). The skill adds additional structured fields (Root cause, Fix, Key insight, etc.) as recommended enhancements for better agent context.
2. Match the actual implementation

```markdown
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!

### Description of Change
[Must match actual implementation]

Expand Down Expand Up @@ -229,11 +223,6 @@ Example: "Before: Safe area applied by default (opt-out). After: Only views impl
Use structured template only when existing description is inadequate:

```markdown
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!

### Root Cause

[Why the bug occurred - be specific about the code path]
Expand Down
4 changes: 0 additions & 4 deletions .github/skills/pr-finalize/references/complete-example.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ This example shows a PR description optimized for future agent success.

## Description
```markdown
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!

### Root Cause

In `MauiView.GetAdjustedSafeAreaInsets()` on iOS, views that don't implement `ISafeAreaView` or `ISafeAreaView2` (such as `ContentPresenter`, `Border`) were falling through to return `baseSafeArea`. This applied full device safe area insets to views that never opted into safe area handling, causing double-padding when used inside ControlTemplates.
Expand Down
2 changes: 2 additions & 0 deletions eng/pipelines/common/ui-tests-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ steps:
- task: PublishBuildArtifacts@1
condition: always()
displayName: publish artifacts
inputs:
artifactName: drop-$(System.StageName)-$(System.JobName)-$(System.JobAttempt)

# Enable Notification Center re-enabled only for catalyst
- ${{ if eq(parameters.platform, 'catalyst')}}:
Expand Down
19 changes: 16 additions & 3 deletions src/Controls/src/Core/Shell/ShellNavigationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -313,14 +313,27 @@ public static void ApplyQueryAttributes(Element element, ShellRouteParameters qu
var mergedData = MergeData(element, filteredQuery, isPopping);

//if we are pop or navigating back, we need to apply the query attributes to the ShellContent
if (isPopping)
if (isPopping && mergedData.Count > 0 )
{
element.SetValue(ShellContent.QueryAttributesProperty, mergedData);
Comment thread
kubaflo marked this conversation as resolved.
}
baseShellItem.ApplyQueryAttributes(mergedData);

// Skip applying query attributes if the merged data is empty and we're popping back
// This respects when user calls query.Clear() - they don't want attributes applied on back navigation
if (mergedData.Count > 0 || !isPopping)
{
baseShellItem.ApplyQueryAttributes(mergedData);
}
}
else if (isLastItem)
element.SetValue(ShellContent.QueryAttributesProperty, MergeData(element, query, isPopping));
{
var mergedData = MergeData(element, query, isPopping);
// Skip setting query attributes if the merged data is empty and we're popping back
if (mergedData.Count > 0 || !isPopping)
{
element.SetValue(ShellContent.QueryAttributesProperty, mergedData);
}
}

ShellRouteParameters MergeData(Element shellElement, ShellRouteParameters data, bool isPopping)
{
Expand Down
167 changes: 167 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue33415.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
using Microsoft.Maui.Controls;
using System.Collections.Generic;

namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.Github, 33415, "ApplyQueryAttributes gets called with empty Dictionary on back", PlatformAffected.All)]
public class Issue33415 : TestShell
{
protected override void Init()
{
// Start on a landing page, not the test page
var landingPage = new ContentPage
{
Content = new Button
{
AutomationId = "NavigateToMainPageButton",
Text = "Navigate to Main Page with Parameters",
Command = new Command(async () =>
{
await Shell.Current.GoToAsync("MainPage", new Dictionary<string, object>
{
{ "TestKey", "TestValue" }
});
})
}
};
Items.Add(new ShellContent { Title = "Landing", Content = landingPage });

// Register the main test page as a route (not starting page)
Routing.RegisterRoute("MainPage", typeof(Issue33415MainPage));
Routing.RegisterRoute("SecondPage", typeof(Issue33415SecondPage));
}
}

public class Issue33415MainPage : ContentPage, IQueryAttributable
{
private readonly Label _statusLabel;
private readonly Label _callCountLabel;
private readonly Button _navigateButton;
private int _applyQueryCallCount = 0;

public Issue33415MainPage()
{
_statusLabel = new Label
{
AutomationId = "StatusLabel",
Text = "Status: Not called",
Margin = new Thickness(10)
};

_callCountLabel = new Label
{
AutomationId = "CallCountLabel",
Text = "Call count: 0",
Margin = new Thickness(10)
};

_navigateButton = new Button
{
AutomationId = "NavigateButton",
Text = "Navigate to Second Page",
Margin = new Thickness(10)
};

_navigateButton.Clicked += OnNavigateClicked;

Content = new StackLayout
{
Padding = new Thickness(10),
Children =
{
new Label
{
Text = "Issue 33415 - ApplyQueryAttributes called with empty Dictionary on back",
FontSize = 16,
FontAttributes = FontAttributes.Bold,
Margin = new Thickness(0, 0, 0, 20)
},
_statusLabel,
_callCountLabel,
_navigateButton
}
};
}

public void ApplyQueryAttributes(IDictionary<string, object> query)
{
_applyQueryCallCount++;
_callCountLabel.Text = $"Call count: {_applyQueryCallCount}";

if (query.Count == 0)
{
_statusLabel.Text = "Status: ApplyQueryAttributes called with EMPTY dictionary";
}
else
{
_statusLabel.Text = $"Status: ApplyQueryAttributes called with {query.Count} parameter(s)";

// According to documentation, calling Clear() should prevent
// ApplyQueryAttributes from being called again on back navigation
query.Clear();
}
}

private async void OnNavigateClicked(object sender, System.EventArgs e)
{
// Navigate to second page - pass through the same parameter for testing
await Shell.Current.GoToAsync("SecondPage", new Dictionary<string, object>
{
{ "TestKey", "TestValue" }
});
}
}

[QueryProperty(nameof(TestKey), "TestKey")]
public class Issue33415SecondPage : ContentPage
{
private readonly Label _receivedLabel;
private string _testKey;

public string TestKey
{
get => _testKey;
set
{
_testKey = value;
_receivedLabel.Text = $"Received: {value ?? "null"}";
}
}

public Issue33415SecondPage()
{
_receivedLabel = new Label
{
AutomationId = "ReceivedLabel",
Text = "Received: null",
Margin = new Thickness(10)
};

var backButton = new Button
{
AutomationId = "BackButton",
Text = "Go Back",
Margin = new Thickness(10)
};

backButton.Clicked += async (s, e) => await Shell.Current.GoToAsync("..");

Content = new StackLayout
{
Padding = new Thickness(10),
Children =
{
new Label
{
Text = "Second Page",
FontSize = 16,
FontAttributes = FontAttributes.Bold,
Margin = new Thickness(0, 0, 0, 20)
},
_receivedLabel,
backButton
}
};
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues
{
public class Issue33415 : _IssuesUITest
{
public override string Issue => "ApplyQueryAttributes gets called with empty Dictionary on back";

public Issue33415(TestDevice device) : base(device) { }

[Test]
[Category(UITestCategories.Shell)]
public void ApplyQueryAttributesShouldNotBeCalledWithEmptyDictionaryOnBack()
{
// Start on landing page - tap to navigate TO MainPage WITH parameters
App.WaitForElement("NavigateToMainPageButton");
App.Tap("NavigateToMainPageButton");

// Wait for main page to appear
App.WaitForElement("StatusLabel");
App.WaitForElement("CallCountLabel");

// Verify ApplyQueryAttributes was called with parameters
var callCountAfterNav = App.FindElement("CallCountLabel").GetText();
var statusAfterNav = App.FindElement("StatusLabel").GetText();
Assert.That(callCountAfterNav, Is.EqualTo("Call count: 1"), "ApplyQueryAttributes should be called once with parameters");
Assert.That(statusAfterNav, Does.Contain("1 parameter"), "Status should indicate parameters were received");

// Navigate to second page
App.Tap("NavigateButton");

// Wait for second page to appear
App.WaitForElement("ReceivedLabel");

// Verify second page received the parameter (passed through from MainPage navigation)
var receivedText = App.FindElement("ReceivedLabel").GetText();
Assert.That(receivedText, Is.EqualTo("Received: TestValue"), "Second page should receive the parameter");

// Go back to main page
App.Tap("BackButton");

// Wait for main page to reappear
App.WaitForElement("StatusLabel");
App.WaitForElement("CallCountLabel");

// BUG: According to documentation, after calling query.Clear() in ApplyQueryAttributes,
// the method should NOT be called again when navigating back.
// However, it IS called with an empty dictionary.
var finalCallCount = App.FindElement("CallCountLabel").GetText();
var finalStatus = App.FindElement("StatusLabel").GetText();

// This assertion will FAIL (demonstrating the bug)
// Expected: Call count should still be 1 (ApplyQueryAttributes should not be called on back)
// Actual: Call count will be 2 (ApplyQueryAttributes IS called with empty dictionary)
Comment on lines +48 to +56

Copilot AI Jan 9, 2026

Copy link

Choose a reason for hiding this comment

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

The comments on lines 54-56 state "This assertion will FAIL (demonstrating the bug)" which is misleading since this test is part of the PR that includes the fix. The test should PASS with the fix applied. Consider updating the comment to reflect that this test verifies the fix works correctly, e.g., "This assertion verifies that ApplyQueryAttributes is NOT called when navigating back after query.Clear()"

Suggested change
// BUG: According to documentation, after calling query.Clear() in ApplyQueryAttributes,
// the method should NOT be called again when navigating back.
// However, it IS called with an empty dictionary.
var finalCallCount = App.FindElement("CallCountLabel").GetText();
var finalStatus = App.FindElement("StatusLabel").GetText();
// This assertion will FAIL (demonstrating the bug)
// Expected: Call count should still be 1 (ApplyQueryAttributes should not be called on back)
// Actual: Call count will be 2 (ApplyQueryAttributes IS called with empty dictionary)
// According to documentation, after calling query.Clear() in ApplyQueryAttributes,
// the method should NOT be called again when navigating back.
// This test verifies that behavior and ensures ApplyQueryAttributes is not invoked with an empty dictionary.
var finalCallCount = App.FindElement("CallCountLabel").GetText();
var finalStatus = App.FindElement("StatusLabel").GetText();
// This assertion verifies that ApplyQueryAttributes is NOT called when navigating back after query.Clear()
// i.e., the call count should still be 1 when returning to the main page.

Copilot uses AI. Check for mistakes.
Assert.That(finalCallCount, Is.EqualTo("Call count: 1"),
"ApplyQueryAttributes should NOT be called when navigating back after query.Clear()");

// If the bug exists, this will show it was called with empty dictionary
if (finalCallCount != "Call count: 1")
{
Assert.That(finalStatus, Is.EqualTo("Status: ApplyQueryAttributes called with EMPTY dictionary"),
"If ApplyQueryAttributes is incorrectly called, it's called with empty dictionary");
}
Comment on lines +59 to +65

Copilot AI Jan 9, 2026

Copy link

Choose a reason for hiding this comment

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

The conditional check at lines 61-65 is redundant. If the assertion at line 57 passes (finalCallCount == "Call count: 1"), this code block will never execute. If the assertion fails, the test will already have failed. Consider removing this conditional block as it serves no purpose.

Suggested change
// If the bug exists, this will show it was called with empty dictionary
if (finalCallCount != "Call count: 1")
{
Assert.That(finalStatus, Is.EqualTo("Status: ApplyQueryAttributes called with EMPTY dictionary"),
"If ApplyQueryAttributes is incorrectly called, it's called with empty dictionary");
}

Copilot uses AI. Check for mistakes.
}
}
}
Loading