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

Fix #2963: Add support for double click events in CEF #3040

Merged
merged 4 commits into from
Aug 15, 2023

Conversation

Lpsd
Copy link
Member

@Lpsd Lpsd commented May 24, 2023

Resolves #2963

As mentioned in the issue description above, double click events previously weren't being sent to CEF.

Test resource:
cef.zip

Usage: start the resource & click your mouse. a status (single or double click) will be displayed on the browser page, as well as in the console

@Lpsd Lpsd added the bug Something isn't working label May 24, 2023
@patrikjuvonen patrikjuvonen changed the title Add support for double click events in CEF Fix #2963: Add support for double click events in CEF May 25, 2023
@lopezloo
Copy link
Member

lopezloo commented May 26, 2023

I think we shouldn't test such things with jQuery. I have modified your script to do not use jQuery and also handle mousedown and mousedown events:

index.html
<html>
    <head>
        <title>CEF Test</title>
    </head>
    <style>
        div {
            width: 100%; 
            color: #ccc; 
            margin-top: 10vh; 
            text-align: center; 
            font-size: 30px
        }
    </style>
    <body style="background: #222;">
        <div>Click anywhere</div>
        <div class="output" style="color: rgb(184, 49, 49); margin-top: 25px;"></div>

        <script>
            function log(message) {
                console.log(message);
                document.querySelector('.output').innerHTML += '<br>' + message;
            }

            addEventListener("mousedown", (event) => {
                log("mousedown @" + new Date().getTime());
            });

            addEventListener("mouseup", (event) => {
                log("mouseup @" + new Date().getTime());
            });

            addEventListener("dblclick", (event) => {
                log("double click @" + new Date().getTime());
            });

            addEventListener("click", (event) => {
                log("click @" + new Date().getTime());
            });
        </script>
    </body>
</html>

@lopezloo
Copy link
Member

lopezloo commented May 26, 2023

Tested and it works but I don't understand why Chromium can't handle it by own?

From https://developer.mozilla.org/en-US/docs/Web/API/Element/dblclick_event:

dblclick fires after two click events (and by extension, after two pairs of mousedown and mouseup events).

Possibly because because it doesn't fire mousedown for next click? This is what double clicking prints on master:

mousedown @1685127605133
mouseup @1685127605179
click @1685127605179
mouseup @1685127605278

This is what slow double clicking prints (on master):

mousedown @1685127891780
mouseup @1685127891887
click @1685127891888
mousedown @1685127892265
mouseup @1685127892368
click @1685127892368

This is what Chrome prints when fast double clicking:

mousedown @1685127953616
mouseup @1685127953662
click @1685127953662
mousedown @1685127953697
mouseup @1685127953758
click @1685127953758
double click @1685127953758

@Lpsd
Copy link
Member Author

Lpsd commented Jun 12, 2023

Can you review your comments above please @lopezloo as I'm fairly certain this PR fixes the issue in the correct way, and is working as intended, as discussed on Discord: https://discord.com/channels/801330706252038164/801411628600000522/1111735006142939136

@lopezloo
Copy link
Member

lopezloo commented Jun 12, 2023

Can you review your comments above please @lopezloo as I'm fairly certain this PR fixes the issue in the correct way, and is working as intended, as discussed on Discord: https://discord.com/channels/801330706252038164/801411628600000522/1111735006142939136

It works but it still looks strange to me that the CEGUI::Window::EventMouseButtonDown event is not being called on double click. Looks like CEGUI thing not really related to this issue. I thought if it would be called then it would be interpreted as double click by Chromium but have tested it and that's not the case so your approach is correct.

Copy link
Member

@botder botder left a comment

Choose a reason for hiding this comment

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

Is the default value necessary? From what I see you never rely on it.

Client/cefweb/CWebView.h Outdated Show resolved Hide resolved
Client/mods/deathmatch/logic/CClientWebBrowser.h Outdated Show resolved Hide resolved
Client/sdk/core/CWebViewInterface.h Outdated Show resolved Hide resolved
botder
botder previously approved these changes Aug 15, 2023
@botder botder merged commit 0daf588 into multitheftauto:master Aug 15, 2023
6 checks passed
@botder botder added this to the 1.6.1 milestone Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CEF : jquery dblclick event not working
3 participants