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

feat(nui/resources): SEND_DUI_KEY_DOWN & SEND_DUI_KEY_UP #2195

Closed
wants to merge 3 commits into from

Conversation

Mycroft-Studios
Copy link

@Mycroft-Studios Mycroft-Studios commented Sep 10, 2023

This PR adds 2 natives:
SEND_DUI_KEY_DOWN
SEND_DUI_KEY_UP

These new natives allow you to inject keyboard events directly into the browser. which was previously:
A. impossible on any external sites
B. very janky and too specific (having to apply the code to each individual input) on internal UI

The need for this was also explained on the forums and I have made a preview video to showcase the type of amazing things these natives can enable.

@blattersturm
Copy link
Contributor

blattersturm commented Sep 10, 2023

In addition to (or rather, instead of?) this I think it'd also make sense to have some functionality to assign NUI focus directly to a specific DUI page, instead of using this to simulate user input in a potentially tacky way. (and inconsistent! as different scripts would do this differently)

The examples you showed do indicate the need rather being to pass user input directly, and these script commands would only make sense to e.g. map controller input to something, but even then that could (and should? especially if users will 'log in to sites' this could be abused to control a browser with user credentials in it) be handled by input mappings rather than making browsers fully controllable.

Similarly, this doesn't handle things like text input across keyboard layouts well either... which is indeed why such commands did not exist in the first place, and adding the ability to accept keyboard input would mean that users can and will log in to sites and therefore script-driven mouse input should be reconsidered for such contexts as well. Thinking about it, maybe users already get logged in randomly to sites like Discord by scanning QR codes as that doesn't need any key input so this really needs some additional thought.

(and similarly, beyond that, some sort of origin display for anything that looks like a credential field to prevent phishing even in full-screen NUI- big rabbit hole...)

@Mycroft-Studios
Copy link
Author

Mycroft-Studios commented Sep 10, 2023

I think it'd also make sense to have some functionality to assign NUI focus directly to a specific DUI page,

I agree with this, and this would be a better way of implementing it but, with the current way NUI focus works, that is not possible. I attempted multiple different ways of adding a "DUI focus", but since NUI focus currently applies focus to only the main Nui tab, and then to the iframe that belongs to that resource, it simply doesn't work with DUI, since each DUI is separate and not contained to a set frame.

(and inconsistent! as different scripts would do this differently)

My implementation specifically focuses on ASCII (which is the same set of characters the FXServer supports anyway), and specifically the JavaScript keys, meaning u can send them either by capturing with NUI, or do manually with IsControlJustPressed.

Similarly, this doesn't handle things like text input across keyboard layouts well either

Why would it not? it generates the scanCode based upon the users layout and "e" is still 101 in ascii, regardless of user layout?

And while I agree with the security concerns, the same can apply for normal NUI, and therefore would be a separate topic in its own right.

Edit: on the subject of consistency and my point on the JS keys.
within the script I used to test this, I just put:

--  send Key Data from Nui to Dui
---@param data table -- the data from the the Nui browser
---@return nil
RegisterNuiCallback("sendKey", function(data, cb)
  cb({})
  if data.key == "Tab" then -- allow exiting of the Dui
    DuiStorage[1]:toggleFocus()
    return
  end
  if data.type == "down" then 
    SendDuiKeyDown(DuiStorage[1].object, data.key) -- tell Dui the key was Pressed.
  else
    SendDuiKeyUp(DuiStorage[1].object, data.key) -- tell Dui the Key Was Released
  end
end)

Within the client side, and then:

window.addEventListener('load', e => {
            window.addEventListener('keydown', function (e) { 
            const options = {
                method: 'post',
                headers: {
                'Content-Type': 'application/json; charset=UTF-8',
                },
                body: JSON.stringify({keyCode: e.keyCode, key: e.key, type: "down"}),
            };
            fetch(`https://${GetParentResourceName()}/sendKey`, options);
        }, false);
          window.addEventListener('keyup', function (e) { 
            const options = {
                method: 'post',
                headers: {
                'Content-Type': 'application/json; charset=UTF-8',
                },
                body: JSON.stringify({keyCode: e.keyCode, key: e.key, type: "up"}),
            };
            fetch(`https://${GetParentResourceName()}/sendKey`, options);
        }, false);
        })

Within an NUI.

@blattersturm
Copy link
Contributor

blattersturm commented Sep 10, 2023

it simply doesn't work with DUI, since each DUI is separate and not contained to a set frame.

This would entail routing input events to that browser instead of the main browser, of course. Something like that was already implemented for the short-lived rootless NUI feature, and currently mpMenu works that way as well.

the same can apply for normal NUI

No, since 'normal NUI' can't have input events passed to it by script.

Similarly, expecting users to write code like what you just posted as example, instead of just calling one routine to route input to a specific DUI instance is just... silly.

@thorium-cfx
Copy link
Contributor

Closing this as I agree with blattersturm's assessment. I've put the request in the backlog and we'll first evaluate the options and use cases.

@thorium-cfx thorium-cfx closed this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants