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

Disable token creation for a bit on keyboard input #3727

Conversation

tarek-y-ismail
Copy link
Contributor

@tarek-y-ismail tarek-y-ismail commented Jan 23, 2025

Fixes apps being activated even while typing.

To test:

  1. Run miral-app
  2. Open a terminal
  3. Run pluma &
  4. Run sleep 2 && pluma
  5. Start typing on your keyboard

Without this fix, pluma manages to request an activation token and request activation with this token between keystrokes. This PR adds a small window (100 ms) after each keystroke where token creation is disabled.

Note: To properly test, the session check here should be removed. We're still discussing whether this check should be included or not as it doesn't make sense for unfocused applications to be able to request tokens and activate themselves, but we haven't seen any applications that behave as expected so far.

@tarek-y-ismail tarek-y-ismail requested a review from a team as a code owner January 23, 2025 17:52
@Saviq
Copy link
Collaborator

Saviq commented Jan 24, 2025

  1. Run pluma &
  2. Run sleep 2 && pluma
  3. Start typing on your keyboard

All this within the terminal?

Without this fix, pluma manages to request an activation token and request activation with this token between keystrokes. This PR adds a small window (100 ms) after each keystroke where token creation is disabled.

Note: To properly test, the session check here should be removed.

That's wrong. Pluma is not focused, it can't get a valid token. There's just no world where that makes sense 🤷. It would mean apps in background could activate themselves completely on their own. Why tokens, then?

I don't think this PR makes sense, typing in the terminal post sleep 2 && pluma should invalidate any tokens the terminal passed on, there's no timeout needed.

@tarek-y-ismail
Copy link
Contributor Author

All this within the terminal?

Yes

That's wrong. Pluma is not focused, it can't get a valid token

Yes, but when you actually discovered the bug, the code to check if the application/session requesting the token was different from the focused one didn't exist on the branch you were building from.

I don't think this PR makes sense, typing in the terminal post sleep 2 && pluma should invalidate any tokens the terminal passed on, there's no timeout needed.

The bug isn't related to who is asking for the token, it's related to when. The whole roundtrip happens between key presses. So we don't have a chance to invalidate tokens.

…-activation-doesnt-invalidate-tokens-on-keyboard-input
@tarek-y-ismail tarek-y-ismail self-assigned this Jan 24, 2025
@AlanGriffiths
Copy link
Collaborator

I too don't understand what this is supposed to address:

  1. An unfocussed application requesting an activation token is pointless, as it is unfocussed it should get a "bogus token" that cannot be used by anyone

  2. An application requesting and using a token itself is only makes sense if one of its windows still has focus when the use happens. Otherwise the token will have been revoked

None of that has to with when the request happens

@Saviq
Copy link
Collaborator

Saviq commented Jan 24, 2025

The bug isn't related to who is asking for the token, it's related to when.

OK so what you're saying is that when the sleep 2 runs out, the terminal gets a separate token for the && pluma and pluma uses that to activate, all between keypresses?

TBH feels to me like the whole sleep 2 && pluma should have just one token - so I'd say that's a bug in the terminal implementation that we shouldn't be working around on the compositor side...?

@AlanGriffiths
Copy link
Collaborator

Oh, so is it the terminal that requests the token?

I guess the meaningful key event for sleep 2 && pluma is the enter. The problem being that, during the sleep, we might see more keypresses and invalidate the token?

@Saviq
Copy link
Collaborator

Saviq commented Jan 24, 2025

The problem being that, during the sleep, we might see more keypresses and invalidate the token?

Not a bug, feature :)

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.

3 participants