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

Updated content script to use the beforeinput event #37

Merged
merged 3 commits into from
Jan 2, 2022
Merged

Conversation

tdulcet
Copy link
Collaborator

@tdulcet tdulcet commented May 6, 2021

  • Updated the content script to use the beforeinput event instead of the keydown and keyup events for autocorrection (see Unable to write tweets on Twitter #4 (comment)).
    • Originally designed as a fix for Unable to write tweets on Twitter #4. While it does not fix that, it may fix other site issues.
    • Improves the performance.
      • Does the autocorrections before the browser modifies the DOM tree (see here).
    • Greatly improves Android capability.
      • Eliminates dependence on the keyboard events which are poorly supported on Android (see Firefox Bug 1104817 and Chrome Bug 118639).
    • Requires Firefox/Thunderbird 87.

@rugk – I think this could be merged sometime after 2021-11-2 when Firefox/Thunderbird 78 will no longer be supported (see #4 (comment)). I would also need to make a companion PR for the Awesome Emoji Picker.

rugk
rugk previously approved these changes May 6, 2021
@tdulcet tdulcet mentioned this pull request Aug 13, 2021
@tdulcet tdulcet marked this pull request as ready for review October 12, 2021 10:25
@tdulcet tdulcet requested a review from rugk October 15, 2021 07:46
rugk
rugk previously approved these changes Oct 23, 2021
Copy link
Owner

@rugk rugk left a comment

Choose a reason for hiding this comment

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

Some minor code-style comments, because that code is really hard to understand. It's likely because it is just a complex piece of code, but we can do better to use understandable variable names etc. 🙂

But basically LGTM.

src/content_scripts/autocorrect.js Outdated Show resolved Hide resolved
src/content_scripts/autocorrect.js Outdated Show resolved Hide resolved
@tdulcet
Copy link
Collaborator Author

tdulcet commented Oct 29, 2021

Some minor code-style comments, because that code is really hard to understand. It's likely because it is just a complex piece of code, but we can do better to use understandable variable names etc. 🙂

Thanks. I made your requested changes in 9d9b403. I know am bad at naming stuff, so please feel free anytime to suggest better names for things. 😉

@rugk rugk merged commit e1e07d2 into main Jan 2, 2022
@rugk rugk deleted the beforeinput branch January 2, 2022 17:23
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.

2 participants