foundryvtt / pf2e

A community contributed game system for Pathfinder Second Edition.
https://foundryvtt.com/packages/pf2e
Apache License 2.0
418 stars 349 forks source link

Pressing enter in a character sheet text field triggers use/consume buttons in unrelated item summaries #10007

Closed tdhsmith closed 1 year ago

tdhsmith commented 1 year ago

This is my own followup research from a comment by /u/440Music on the PF2e subreddit:

When typing +X and hitting Enter, in the XP space, Foundry will auto-use items. It will burn scrolls, consumables, etc., and it entirely removes the object from your inventory (rather than setting its count to 0) so you have to look them up again. I have tried making sure that non-inventory tabs are open when adding XP only for it to not matter and burn random items anyway.

From my own testing I can confirm this happens for various text input fields such as the Hit Points entries in the sidebar, and can even trigger buttons after their summaries are collapsed or the overall sheet tab has changed. (I assume they are still attached to the DOM once they've been opened once?)


Since the character sheet overall is a <form>, pressing Enter in any control is likely triggering the implicit submission behavior for that form, which fires a synthetic click event on the first submit button in tree order, in turn triggering ConsumablePF2e.consume.

It would be sufficient to change these buttons from the default submit to type="button" but I'm not sure if that submission behavior is desirable elsewhere. You might also be able to detect these synthetic events but I don't know if there's a consistent or standards compliant way to do so -- certainly they seem to have some bogus data though like clientX === 0. Probably other options too like adding extra forms to the tree?

I'm happy to make a PR but was a little cautious about the moving parts and preferences here for my first contribution! 😆

tdhsmith commented 1 year ago

Ok I got over my fear. Looks like the vast majority of buttons in templates already had types defined. I did a search for any others that were missing. Found 3 instances in the spell details window, and confirmed that these all had inappropriate/confusion submission behavior so I added them as well.

Let me know if I should address anything else here. It's a super brief submission but I know getting contributors on the same page is important even for small stuff!