[PS-1160] re-order autofill events

[TomK]

Type of change

- [X] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Address #3141

Code changes

• apps/browser/src/content/autofill.js I have re-ordered the events that are fired so the value of the element is correct at the time of each event.

Before you submit

- [x] I have checked for **linting** errors (`npm run lint`) (required)
- [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required)
- [ ] This change requires a **documentation update** (notify the documentation team)
- [ ] This change has particular **deployment requirements** (notify the DevOps team)

[CLAassistant]

All committers have signed the CLA.

[bitwarden-bot]

Thank you for your contribution! We've added this to our internal Community PR board for review. ID: PS-1160

[dbosompem]

Hi @TomK , just a little update on your PR. The team is taking a look at it currently, but we intend to handle this with caution. We agree your PR addresses the issue, but what we would want to look into further is whether it breaks something else, which is even more important. We'd have to make time to evaluate this change with a lot of testing efforts too. Could take some time, but just keeping you in the loop.

[trmartin4]

Hi @TomK. I am an Engineering Manager at Bitwarden. We are evaluating this fix and attempting to verify that there are no unexpected side-effects for other uses of our autofill logic. As you're probably aware, the autofill process has been built iteratively over the years to try to handle different permutations of fields and events.

I was wondering if you could go into a little more detail about how you decided to alter the event order to add the "keyup" event after the setting of the field back to its original value. We want to make sure we understand the intent so we can document it.

[TomK]

Hi @trmartin4 The order of events was based on the feedback from the codepen I wrote: https://codepen.io/oridan/pen/qBomOpM

It appears that under normal keyboard entry the browser recognises the element value as being updated after keydown, but before keyup.

event order	event name	element value
1	focus
2	focusin
3	keydown
4	keypress
5	beforeinput
6	input	a
7	keyup	a
8	change	a
9	blur	a
10	focusout	a

so, i reordered the events to match this finding. Setting el.value immediately before the input event (where this is triggered). In the case of function setValueForElement, el.value is set before keyup for consistency with this.

I did not add a beforeinput trigger as it seemed out of scope for this PR, but perhaps it should be considered?