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

Correct issues with ApiInformation implementation #2305

Conversation

rjmurillo
Copy link
Contributor

@rjmurillo rjmurillo commented Jul 17, 2018

Issue: #2306

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

Depending on OS/Windows SDK installed, WebView may throw unhandled MissingMethodException.

What is the new behavior?

.NET applications use a concept of Union WinMD, which is the union of all known types that exist in the Windows SDK that correspond to the MaxVersionTested setting of the app. If you're running the app on a down-level platform, ApiInformation will tell you the API doesn't exist, but .NET can still JIT methods based on the Union WinMD and perform some reflection tasks. If you actually try to call the API you will get a MissingMethodException at runtime because the API doesn't really exist.

A different problem can occur if you include a higher-versioned .NET library inside a lower-versioned app and try to run it on the higher-versioned build of the OS. In this case, ApiInformation will succeed because the type exists in the system metadata, but .NET will throw a MissingMethodException at runtime because the type didn't exist in the Union WinMD used to build the app.

The change includes a safer wrapper around ApiInformation methods catching the MissingMethodException

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

@GraniteStateHacker, @purani FYI
Resolves #2306. Requires #2309 for tests to execute

.NET applications use a concept of Union WinMD, which is the union of all known types that exist in the Windows SDK that correspond to the MaxVersionTested setting of the app. If you're running the app on a down-level platform, ApiInformation will tell you the API doesn't exist, but .NET can still JIT  methods based on the Union WinMD and perform some reflection tasks. If you actually try to call the API you will get a MissingMethodException at runtime because the API doesn't really exist.

A different problem can occur if you include a higher-versioned .NET library inside a lower-versioned app and try to run it on the higher-versioned build of the OS. In this case, ApiInformation will succeed because the type exists in the system metadata, but .NET will throw a MissingMethodException at runtime because the type didn't exist in the Union WinMD used to build the app.
@rjmurillo rjmurillo added bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ win 3️⃣2️⃣ WebView 🖥️ WPF and WinForms WebView labels Jul 17, 2018
@rjmurillo rjmurillo added this to the 4.0 milestone Jul 17, 2018
@rjmurillo rjmurillo self-assigned this Jul 17, 2018
@rjmurillo rjmurillo requested a review from nmetulev July 17, 2018 23:48
return string.Empty;
return ApiInformationExtensions.ExecuteIfPropertyPresent(
WinRtType,
"Partition",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UserAgent?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be replaced to, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was in 193077d

The UserAgent property mistakenly used Partition property
ApiInformationExtensions.ExecuteIfEventPresent(
WinRtType,
"LostFocus",
() => { _webViewControl.GotFocus -= OnLostFocus; });
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be LostFocus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 4bc5d4c

ApiInformationExtensions.ExecuteIfEventPresent(
WinRtType,
"LostFocus",
() => { _webViewControl.GotFocus += OnLostFocus; });
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be LostFocus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 4bc5d4c

LostFocus event handler was accidentially wired up to GotFocus event.
@azchohfi azchohfi merged commit 11bac79 into CommunityToolkit:master Jul 23, 2018
@rjmurillo rjmurillo deleted the WebView/WebViewControlProcessOptions_put_UserAgent branch July 25, 2018 15:27
@rjmurillo rjmurillo restored the WebView/WebViewControlProcessOptions_put_UserAgent branch July 25, 2018 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ WebView 🖥️ WPF and WinForms WebView win 3️⃣2️⃣
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebView: Unhandled MIssingMethodException causing crash
3 participants