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

Unable to write tweets on Twitter #4

Open
rugk opened this issue Nov 28, 2020 · 21 comments
Open

Unable to write tweets on Twitter #4

rugk opened this issue Nov 28, 2020 · 21 comments
Labels
bug Something isn't working help wanted Extra attention is needed site-issue A problem with/issue about a specific site

Comments

@rugk
Copy link
Owner

rugk commented Nov 28, 2020

Affected website: https://twitter.com/

Bug description

When I try to write a tweet or reply this really seems to fail and reset’s my cursor position to the beginning again.
It seems to get scrambled. (no, no 🍳)

Steps to reproduce

  1. Go to https://twitter.com/ and login.
  2. Write/compose a new tweet at the top.

Screencast

image

text:

I try to write some example text.

Somtimes it also ends like this (likely related, could not see it happening without this add-on, this is in a reply):
image

Actual behavior

This add-on should not interfere with what I write.

System

Operating system and version: Linux/Fedora 33, GNOME
Browser and version: Firefox 83
Add-on version: 733bd33

Possible solution

I have no real idea what this might be related to.

@rugk rugk added bug Something isn't working site-issue A problem with/issue about a specific site labels Nov 28, 2020
@rugk rugk added this to the next milestone Nov 28, 2020
@rugk rugk changed the title Twitter messages unable to https://twitter.com/ Unable to write Tweets on Twitter Nov 28, 2020
@rugk rugk changed the title Unable to write Tweets on Twitter Unable to write tweets on Twitter Nov 28, 2020
@rugk
Copy link
Owner Author

rugk commented Nov 28, 2020

Ah, BTW, the Unicode font and casing features do work on the page for some strange reason.

@tdulcet
Copy link
Collaborator

tdulcet commented Nov 29, 2020

I have no real idea what this might be related to.

This issue is in the getCaretPosition() function here. There is no official way to get the caret position in ContentEditable elements. This block is a hack, which clones the element, inserts the null character, determines the index and then removes the null character. I am guessing that Twitter does not like the null character, but I tried with other nonprinting characters and had the same issue.

@tdulcet tdulcet added the help wanted Extra attention is needed label Nov 29, 2020
@tdulcet
Copy link
Collaborator

tdulcet commented Mar 24, 2021

A possible solution would be to use the new beforeinput event and getTargetRanges() method to get the caret position in ContentEditable elements., but they both require Firefox 87.

@rugk
Copy link
Owner Author

rugk commented Mar 24, 2021

If the change is backwards-compatible (and I think it can be made like that), I see nothing wrong with that. Sure, go for it, if you can improve/fix that! (And don't forget to use feature detection instead of browser detection, if possible. 😃)

@tdulcet
Copy link
Collaborator

tdulcet commented Mar 25, 2021

Hmm, I am not sure how we could make it backwards compatible... The needed getTargetRanges() method is only available with the beforeinput event and they both require the Firefox 87 (the former requires Chrome 60). The interface of the new beforeinput event and the existing keydown/keyup events are also very different, so it would likely require a lot of code duplication to support them both and we would then still need to find a workaround for that code block to support older browsers...

@dvergeylen
Copy link

Hmm, I am not sure how we could make it backwards compatible... The needed getTargetRanges() method is only available with the beforeinput event and they both require the Firefox 87 (the former requires Chrome 60). The interface of the new beforeinput event and the existing keydown/keyup events are also very different, so it would likely require a lot of code duplication to support them both and we would then still need to find a workaround for that code block to support older browsers...

I think this is a no brainer: according to Firefox release schedule, latest version of Firefox-ESR (78.15) is planned for 2021-09-07, starting 2021-10-05 every released Firefox version will be 91+, thus supporting getTargetRanges(). According to caniuse, very few people lack behind more than one year old release... Why should that be backward compatible 🤔?

@tdulcet
Copy link
Collaborator

tdulcet commented Mar 25, 2021

Why should that be backward compatible 🤔?

@rugk requested above that it be backward compatible. Anyway, October is over six months away and we would like to officially release this add-on to AMO, ATN and the Chrome Web Store as soon as possible, so it would obviously be preferable that it supported Firefox ESR 78 and especially Thunderbird 78. In addition, any change made to fix this issue will also need to be made in my companion PR for the Awesome Emoji Picker (rugk/awesome-emoji-picker#93), which still supports Firefox 63, so requiring 87 would likely be a nonstarter...

@dvergeylen
Copy link

Correct, thanks for the explanation 👍

@tdulcet
Copy link
Collaborator

tdulcet commented Apr 6, 2021

@rugk - Hmm, there is probably an easy solution/workaround for this issue that does not require Firefox 87, but I have not been able to find it yet... I am open to ideas if anyone has any. 🤔

There are dozens of posts on Stack Overflow type websites on how to get the caret position in ContentEditable elements. I tried most of them when I initially implemented this (rugk/awesome-emoji-picker#93) and they all either did not work or caused major website breakage. I ended up having to combine several of the solutions to produce the getCaretPosition() function that is now in this add-on, although the issue seems to be from the code I adapted from here, which is incompatible with Twitter's WYSIWYG editor:

const temp = document.createTextNode("\0");
range.insertNode(temp);
const caretposition = target.innerText.indexOf("\0");
temp.parentNode.removeChild(temp);

@rugk
Copy link
Owner Author

rugk commented May 3, 2021

Hmm, there is probably an easy solution/workaround for this issue that does not require Firefox 87, but I have not been able to find it yet... I am open to ideas if anyone has any. thinking

Honestly, I'd be okay, if we require Firefox 87. It works in Firefox stable then and this is a new add-on, so nobody will complain that it does not work in their browser "anymore". 😉

From an UX perspective (having used that add-on in practise for some time), this is really an annoying thing.

Okay, it's quite funny actually everytime I try to write a tweet I get complete garbage and think I'm writing in a fantasy language or Twitter wants to troll me and then it takes some seconds until I actually remember this is caused by this add-on here. I then need to disable it, write my tweet and then re-enable it.
I am actually not that often on Twitter though.

from #34

So if that problem would really be limited to Twitter, IMHO it would also be fine for 1.0 to hardcode and exclusion and do not run the add-on on Twitter at all.

@tdulcet
Copy link
Collaborator

tdulcet commented May 4, 2021

Honestly, I'd be okay, if we require Firefox 87. It works in Firefox stable then and this is a new add-on, so nobody will complain that it does not work in their browser "anymore". 😉

Well, any fix for this would need to be backported to rugk/awesome-emoji-picker#93. Regardless, I attempted to update my local copy of the extension to use the beforeinput event and getTargetRanges() method. The beforeinput event is definitely a big improvement in turns of performance and Firefox for Android capability. I will for sure create a draft PR for that so people can see the changes and try it out. However, after further researching the getTargetRanges() method, there unfortunately still does not seem to be a way to get the caret position in ContentEditable elements in the format we need it. It actually is not much different than the getRangeAt() method that the extension currently uses, so I am not sure what all the hype around it was.

The main issue with it and most of the other solutions is that they provide the caret position with respect to the textContent and for the autocorrect to work correctly, we need it with respect to the innerText. I am still open to ideas if anyone has any. 🤔

So if that problem would really be limited to Twitter, IMHO it would also be fine for 1.0 to hardcode and exclusion and do not run the add-on on Twitter at all.

I think this should only be done as an extreme last resort, since the issue could very well affect other websites. It would just require adding Twitter to the exclude_matches in the Firefox and Chrome manifest.json files (see here).

@rugk
Copy link
Owner Author

rugk commented May 4, 2021

Well, any fix for this would need to be backported to rugk/awesome-emoji-picker#93.

Hmm yeah, maybe it would make sense to make a common library at some point of time. 🙂

@rugk
Copy link
Owner Author

rugk commented Aug 12, 2021

I still have this as a blocker for the next beta release (see the milestone)… as the only issue now.
Maybe just because I am biased and affected by this issue, so well… 😉 Anyway, this is problematic and I consider it a quality thing to fix/handle/provide a workaround/setting for the user or whatever… at least for a stable release.

So hmm… what is the current status here, @tdulcet? Any chance to find a real fix or workaround, or should we chase a way by creating an exclusion list – possibly even hardcoded for now.
I don't recall it all, so a summmary would be great. 🙂

@tdulcet
Copy link
Collaborator

tdulcet commented Aug 13, 2021

So hmm… what is the current status here, @tdulcet? Any chance to find a real fix or workaround, or should we chase a way by creating an exclusion list – possibly even hardcoded for now.
I don't recall it all, so a summmary would be great. 🙂

See my previous post above for a good summary of the current state of this issue.

I have been trying different possible solutions/workarounds as I find them, but I have not been able to find a real fix yet... As I said, I am open to ideas if anyone has any. As with #55, we will likely need someone who is an expert on WYSIWYG editors. We could always ask on Mozilla's Discourse or Stack Overflow, but I think our chances of getting an acceptable solution there would be very low.

Regarding an exclusion list, I can add that if needed, just let me know.

@rugk
Copy link
Owner Author

rugk commented Aug 14, 2021

So let's tackle that more general in #62

@rugk
Copy link
Owner Author

rugk commented Aug 19, 2021

TODO:

  • find out library used here

@rugk
Copy link
Owner Author

rugk commented Sep 15, 2021

@tdulcet Another update here and they now suggest the solution we have – at least it uses this null-character?

However, ah, now I read there is a difference:

I then use this snippet to set the caret's position, which should be the fix to Twitter

Do you want to test that new solution? 🤔

@tdulcet
Copy link
Collaborator

tdulcet commented Sep 16, 2021

Do you want to test that new solution? 🤔

Yeah, I did, see #62 (comment):

I created a draft PR with the changes from their updated answer: #69. It now seems to return the correct carrot position, but it still does not work on Twitter. 😕

@rugk
Copy link
Owner Author

rugk commented Nov 6, 2021

Twitter also does clever font tricks to make :) appear more like an emoji by itself. This may be relaterd to this problem, but may actually also not be, because it's not CSS/JS stuff, but just stupid font things (which is great BTW; but off-topic): https://chaos.social/@rixx/107231357100411285

@tdulcet
Copy link
Collaborator

tdulcet commented May 19, 2023

I believe this proposed new API might resolve the issue for us: https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/main/RangeInnerText/explainer.md

@tdulcet
Copy link
Collaborator

tdulcet commented Jun 16, 2024

I just filed Bug 1902858 on BMO to request new WebExtension APIs to support this use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed site-issue A problem with/issue about a specific site
Projects
None yet
Development

No branches or pull requests

3 participants