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

[Problem/Bug]: Inappropriate use of CHECK_FEATURE_RETURN in InitializeFindOptions sample code #5064

Open
ajtruckle opened this issue Jan 28, 2025 · 7 comments
Assignees
Labels
bug Something isn't working tracked We are tracking this work internally.

Comments

@ajtruckle
Copy link

ajtruckle commented Jan 28, 2025

What happened?

The CHECK_FEATURE_RETURN macro can't be used in the proposed InitializeFindOptions method:

wil::com_ptr<ICoreWebView2FindOptions> AppWindow::InitializeFindOptions(const std::wstring& findTerm)
{
    // Query for the ICoreWebView2Environment5 interface.
    auto webView2Environment5 = m_webViewEnvironment.try_query<ICoreWebView2Environment5>();
    CHECK_FEATURE_RETURN(webView2Environment5);

    // Initialize Find options
    wil::com_ptr<ICoreWebView2FindOptions> find_options;
    CHECK_FAILURE(webView2Environment5->CreateFindOptions(&find_options));
    CHECK_FAILURE(find_options->put_FindTerm(findTerm.c_str()));

    return find_options;
}

This is because it returns true upon success. So I adapted by adding a new macro:

#define CHECK_FEATURE_RETURN_NULL(feature) { if (!feature) { FeatureNotAvailable(); return nullptr; } }

The sample code should be revised.

Importance

Important. My app's user experience is significantly compromised.

Runtime Channel

Prerelease (Edge Canary/Dev/Beta)

Runtime Version

134.0.3101.0 canary

SDK Version

1.0.3079 prerelease

Framework

Win32

Operating System

Windows 11

OS Version

10.0.26100 24H2

Repro steps

Try to use the sample in the Find documentation for Win32.

Repros in Edge Browser

No, issue does not reproduce in the corresponding Edge version

Regression

No, this never worked

Last working version (if regression)

No response

@ajtruckle ajtruckle added the bug Something isn't working label Jan 28, 2025
@prija-microsoft prija-microsoft added the tracked We are tracking this work internally. label Jan 29, 2025
@ajtruckle
Copy link
Author

Any update?

@chetanpandey1266
Copy link

Thanks for pointing it out. But this is an outdates branch so please don't refer to it. Closing the issue. Please reopen if you have more to discuss

@ajtruckle
Copy link
Author

This ticket should not be closed. Even in the new documentation:

https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2experimentalfind?view=webview2-1.0.3079-prerelease#start

It is still using incompatible code:

bool AppWindow::Start(const std::wstring& searchTerm)
{
    auto webView2Experimental29 = m_webView.try_query<ICoreWebView2Experimental29>();
    CHECK_FEATURE_RETURN(webView2Experimental29);

That needs to be addressed. Understandably we as users can't keep up with beta copies of the documentation so it is appreciated if our feedback is applied accordingly and carried across. After-all, it is marked as "tracked" and yet not completed, but closed.

Don't have rights to re-open.

@chetanpandey1266
Copy link

The function mentioned in the official documentation : AppWindow::Start has the return type of bool and CHECK_FEATURE_RETURN also returns bool. Are you pointing to some other incompatibility?
I'll reopen the issue on your request

@ajtruckle
Copy link
Author

The code I stated has nothing to do with Start in itself:

auto webView2Experimental29 = m_webView.try_query();
CHECK_FEATURE_RETURN(webView2Experimental29);

webView2Experimental29 Is not a bool. So this is invalid:

CHECK_FEATURE_RETURN(webView2Experimental29);

We are not checking for a bool but a null pointer. As I originally stated.

@chetanpandey1266
Copy link

The type of webview2Experimental29 doesn't matter here, because that's what we are passing to the macro. The type that is returned from the macro matters.

In the function, that you mentioned above, was returning wil::com_ptr which is why CHECK_FEATURE_RETURN ( which returns bool ) shouldn't be used there and your macro that is CHECK_FEATURE_RETURN_NULL would work.

But in the documentation the function ( that is AppWindow::Start ) is returning bool and hence CHECK_FEATURE_RETURN works there.

I hope I was able to make things clear. Please tell me if I'm wrong in understanding your concern

@ajtruckle
Copy link
Author

ajtruckle commented Feb 26, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tracked We are tracking this work internally.
Projects
None yet
Development

No branches or pull requests

3 participants