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

MapControl in release not working without a call to InitializeMap #9324

Closed
nickrandolph opened this issue Feb 6, 2024 · 27 comments
Closed
Labels
fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. needs-triage Issue needs to be triaged by the area owners team-Controls Issue for the Controls team
Milestone

Comments

@nickrandolph
Copy link

nickrandolph commented Feb 6, 2024

Describe the bug

Is the MapControl in the experimental release supposed to be working? All I'm seeing is a blue screen

image

Steps to reproduce the bug

New WinUI application
Update to experimental release
Use following XAML in MainWindow

<Window
    x:Class="MapSample.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:MapSample"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d">

    <Grid>
        <MapControl MapServiceToken="XXXXX" />
    </Grid>
</Window>

Expected behavior

Shows a map, or at least throws a meaningful error as why it's not showing anything

Screenshots

image

NuGet package version

1.5.240124002-experimental2

Packaging type

Tried both Packaged and Unpackaged

Windows version

Windows 11

IDE

VS 2022 (Version 17.9.0 Preview 4.0)

@dotMorten
Copy link
Contributor

I hit the same problem. Tried both the token you'd use with UWP mapcontrol and the one you'd use with Azure maps (the latter which it looks like this control is just wrapping the Azure Maps Javascript control in a webview).

@dotMorten
Copy link
Contributor

dotMorten commented Feb 6, 2024

Looks like initializeMap isn't called. I got things working (thank you open source!!!) with this workaround:

 public MainWindow()
 {
     this.InitializeComponent();
     mapControl.MapServiceToken = mapServiceToken;
     mapControl.Loaded += MapControl_Loaded;
 }

 private void MapControl_Loaded(object sender, RoutedEventArgs e)
 {
     var webView = VisualTreeHelper.GetChild(mapControl, 0) as WebView2;
     webView.NavigationCompleted += WebView_NavigationCompleted;
 }

 private async void WebView_NavigationCompleted(WebView2 sender, Microsoft.Web.WebView2.Core.CoreWebView2NavigationCompletedEventArgs args)
 {
     sender.NavigationCompleted -= WebView_NavigationCompleted;
     var result = await sender.ExecuteScriptAsync($"initializeMap(0,0,\"{mapControl.MapServiceToken}\");");
 }

I'm honestly not quite sure why this fixes it - it seems like this is exactly what the code is already doing.

@hez2010
Copy link

hez2010 commented Feb 7, 2024

BTW I would like to expect we can have a native MapControl just like what the UWP MapControl was, instead of the current WebView based garbage.

@devsko
Copy link

devsko commented Feb 7, 2024

It kind of works for me after upgrading to version 1.5.240205001-preview1. Unfortunately, this kind of map is far from being an adequate replacement for the UWP MapControl.

@eduardobragaxz
Copy link

eduardobragaxz commented Feb 8, 2024

BTW I would like to expect we can have a native MapControl just like what the UWP MapControl was, instead of the current WebView based garbage.

I was taken aback by that. It makes no sense. Oh boy can't wait to see how they bring back InkCanvas now...

@dotMorten
Copy link
Contributor

dotMorten commented Feb 8, 2024

@eduardobragaxz It's probably worth opening an issue specifically discussing this rather than bringing it up on this unrelated issue, since several are now mentioning it here

@eduardobragaxz
Copy link

@dotMorten you're right, my bad. I opened a discussion in the WinUI repo: #9323

@ranjeshj ranjeshj transferred this issue from microsoft/WindowsAppSDK Feb 8, 2024
@ranjeshj ranjeshj added the team-Controls Issue for the Controls team label Feb 8, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Feb 8, 2024
@bkudiess
Copy link
Contributor

bkudiess commented Feb 9, 2024

I tested with preview 1 and it works fine, please reopen the issue with a repro app if you can still repro. Here is the code I used: <MapControl x:Name="myMap" Width="800" Height="600" InteractiveControlsVisible="True" MapServiceToken = "foobar" />
This is the link to get a MapServiceToken: https://learn.microsoft.com/en-us/azure/azure-maps/quick-demo-map-app#get-the-subscription-key-for-your-account

@bkudiess bkudiess closed this as completed Feb 9, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the needs-triage Issue needs to be triaged by the area owners label Feb 9, 2024
@nickrandolph
Copy link
Author

Still able to reproduce this issue:
image

Target framework: net8.0-windows10.0.19041.0

Package references

    <PackageReference Include="Microsoft.WindowsAppSDK" Version="1.5.240205001-preview1" />
    <PackageReference Include="Microsoft.Windows.SDK.BuildTools" Version="10.0.22621.2428" />

With the workaround @dotMorten suggested, the map shows. Otherwise, just a blue screen (worse if running unpackaged).

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Feb 9, 2024
@dotMorten
Copy link
Contributor

dotMorten commented Feb 9, 2024

I'm not seeing Preview1 work any better either without my workaround. both x86 and x64 tested, with and without debugger attached, and both C# and C++. I think this issue should be reopened.

@ranjeshj ranjeshj reopened this Feb 9, 2024
@ranjeshj
Copy link
Contributor

ranjeshj commented Feb 9, 2024

@dotMorten can you share the repro project you have?

@ranjeshj ranjeshj added needs-author-feedback Asked author to supply more information. and removed needs-triage Issue needs to be triaged by the area owners labels Feb 9, 2024
@dotMorten
Copy link
Contributor

Here you go:
MapLoadRepro.zip

@bkudiess
Copy link
Contributor

@dotMorten can you send more information regarding hardware/os version you tried on? Just tested this repro app and it worked for me.

@dotMorten
Copy link
Contributor

dotMorten commented Feb 21, 2024

@bkudiess I've tried on 4 different systems, and they all exhibit the same behavior, including on a Surface Laptop Studio 2 and Surface Laptop 5, both with latest Windows 11, and two desktop PCs (both Win11). I don't think this is a hardware/OS specific. Also reported by someone else here

@dotMorten
Copy link
Contributor

dotMorten commented Mar 1, 2024

Still broken in the final v1.5 release. I've seen this issue referred to in other discussions as well. Surprised this made it to release. My workaround above does still seem to do the trick, but it's by no means ideal.

@ranjeshj
Copy link
Contributor

ranjeshj commented Mar 1, 2024

Still broken in the final v1.5 release. I've seen this issue referred to in other discussions as well. Surprised this made it to release. My workaround above does still seem to do the trick, but it's by no means ideal.

@dotMorten we are still working on getting a repro. Will service a fix to 1.5 once the issue is understood and we have a fix.

@dotMorten
Copy link
Contributor

Since my workaround essentially does what your code does but ensures it delays until webview is ready I’m guessing there’s race conditions at play. But then again I’ve never seen it work, and I don’t know of anyone outside Microsoft who has

@ranjeshj ranjeshj changed the title MapControl in experimental release not working MapControl in release not working without a call to InitializeMap Mar 1, 2024
@ranjeshj
Copy link
Contributor

ranjeshj commented Mar 1, 2024

Agreed that this needs to be addressed. Does sound like a race condition. Just tried out a simple example

 <Grid>
     <MapControl MapServiceToken=..  />
 </Grid>

and that worked out of the box for me on 1.5 stable. Will ask a few more folks in the team to give it a try to see if someone can repro.

image

If there is some pattern where it works vs where it does not work, that would be helpful.

There are also a couple of issues here. If the key is not correct, then we also get the blue screen, but this seems to be different from that since it works with your workaround for you which means the key is fine.

@dotMorten
Copy link
Contributor

Yes this isn't a keys issue. The blue background is caused by this: #9377 I'm guessing someone forgot to remove some early debugging style?
It would be nice if there was a better error if there was a keys issue, like a callback or message on the map area that the key is invalid or the very least a message in the output window.

@riverar
Copy link
Contributor

riverar commented Mar 1, 2024

I can reproduce this.

The root cause is this check for an alpha-numeric token. There should be no such check and instead rely on error propagation from the server side. (My token has a hyphen in it.)

bool MapControl::IsValidMapServiceToken() {
return !MapServiceToken().empty() && std::all_of(MapServiceToken().begin(), MapServiceToken().end(), ::isalnum);
}

@dotMorten
Copy link
Contributor

dotMorten commented Mar 1, 2024

LOLOLOL. I looked at that but didn't think too much of it other than it was an odd check to see if it is valid. I have an underscore in mine.
On the upside, this is a win for debuggable sourcecode.
I think this and this gave me a bigger pause/understanding.

@riverar
Copy link
Contributor

riverar commented Mar 1, 2024

Workaround: Ask users to recycle their keys until an alpha-numeric-only variant appears. This is of course unacceptable from a security perspective, so hopefully the team is now paying attention to this issue and will service this ASAP.

@gegao18
Copy link

gegao18 commented Mar 5, 2024

Thank you for reporting this issue (and huge thanks to @riverar for debugging this). We've checked in a fix for our first 1.5 servicing release.

IsValidMapServiceToken is still needed since the token is passed into ExecuteScriptAsync in a string literal. We've updated it to just check for a double quote instead:

bool MapControl::IsValidMapServiceToken()
{
    // This token is passed as a JavaScript param in double quotes. Make sure the token itself doesn't contain any.
    return !MapServiceToken().empty()
        && std::all_of(MapServiceToken().begin(), MapServiceToken().end(), [](wchar_t i){return i != '"';});
}

@gegao18 gegao18 closed this as completed Mar 5, 2024
@codendone codendone added this to the WinAppSDK 1.5 milestone Mar 5, 2024
@codendone codendone added the fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. label Mar 5, 2024
@riverar
Copy link
Contributor

riverar commented Mar 5, 2024

@gegao18 This change will fail on ", which could be another valid token character (now or in the future). It also misses characters such as \ which will break ExecuteScriptAsync. I would instead consider encoding the input token and skip the testing altogether.

Something like...

- auto token = IsValidMapServiceToken() ? MapServiceToken() : L"";
- auto parameters =  std::format(L"{},\"{}\"", pos, token);
+ auto token = JsonValue::CreateStringValue(MapServiceToken());
+ auto script = std::format(L"initializeMap({},'{}');", pos, token.Stringify().c_str());

  if (auto webView = m_webView.get())
  {
      auto core = webView.CoreWebView2();
      // Initializing Azure Maps on WebView
      // params: longitude, latitude, mapServiceToken
-     returnedValue = co_await core.ExecuteScriptAsync(L"initializeMap(" + parameters + L");");
+     returnedValue = co_await core.ExecuteScriptAsync(script);

      for (uint32_t i = 0; i < Layers().Size(); i++)
      {
          winrt::MapElementsLayer layer = Layers().GetAt(i).as<winrt::MapElementsLayer>();
          OnLayerAdded(layer);
      }
  }

Thanks for the quick triage!

@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Mar 5, 2024
@riverar
Copy link
Contributor

riverar commented Mar 26, 2024

@gegao18 Ping

@gegao18
Copy link

gegao18 commented Mar 26, 2024

Thanks for pinging. For 1.5.1 we made the quick fix that accepts all but ". For 1.6 we have the full fix that passes an encoded token into the map.

@riverar
Copy link
Contributor

riverar commented Mar 27, 2024

@gegao18 Perfect, thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed-internally This bug has been fixed, and the fix will be shipped in the next version of WinUI 3. needs-triage Issue needs to be triaged by the area owners team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

10 participants