guoguo12 / messenger-shortcuts

Keyboard shortcuts for the Facebook Messenger web app (messenger.com).
https://chrome.google.com/webstore/detail/keyboard-shortcuts-for-me/elgfaolomlhhmppjdicpgpmglkllebfb?hl=en-US&gl=US
19 stars 9 forks source link

Uncaught TypeError - extension is not language-independent #24

Closed LoogleCZ closed 4 years ago

LoogleCZ commented 4 years ago

Bug description

I've installed extension now and I see following error:

messenger-shortcuts.js:141 Uncaught TypeError: Cannot read property 'fireEvent' of null
    at fireMouseEvent (messenger-shortcuts.js:141)
    at click (messenger-shortcuts.js:152)
    at toggleInfo (messenger-shortcuts.js:181)
    at HTMLBodyElement.document.body.onkeydown (messenger-shortcuts.js:101)

Browser:

Google Chrome Version 86.0.4240.75

Further investigation

After take a look into the script, you can that the cause is in method event methods (eg. toggleInfo). It searches for element to click on by arial-label and english content of it (eg. getByAttr('a', 'aria-label', 'Conversation Information')). And since I use messenger.com in Czech language mutation, the string to find element by should be Informace o konverzaci.

Maybe it would be better to search the element by another selector, for example I see data-testid which seems to be pretty universal:

<a aria-expanded="true" aria-label="Informace o konverzaci" class="_30yy" role="button" title="Informace o konverzaci" data-testid="info_panel_button" href="#">
     ...
</a>

Proposal

Search for elements by language independent identifier. If not possible to find given element, do not fail by uncaught exception, but implement guard checks in order to fail silently.

Expected behaviour

Addon works for every language mutation

guoguo12 commented 4 years ago

Thanks for reporting this! I agree using data-testid would be better here. Can you make a PR? Or I can fix it later this week.

LoogleCZ commented 4 years ago

Well, after trying to gather all ids for all actions taht can be triggered by this extension, I've found out that not all elements have the id attribute or data-testid. Thus it is more complicated to fix global solution as there is no id and classes seems to be pretty chaotic to use.

For example "Send a Like" button is displayed as

<a aria-label="Poslat To se mi líbí" class="_5j_u _30yy _4rv9 _6ymq _7kpj" role="button" title="Poslat To se mi líbí" href="#">
....
</a>

What came up to my mind when thinking about solution to this, is maybe to be able to switch language based on the lang attribute in the <html> tag.

So structure of the extension would be:

Content of lang/*.json would look like:

{
    "send_a_like": "Poslat To se mi líbí",
    "new_message": "Nová zpráva",
    "conversation_information": "Informace o konverzaci",
    ...
}

and the main script would then use strings defined by language specific json.

@guoguo12 What do you think? I'll be able to work on this maybe next week so I can send PR later

LoogleCZ commented 4 years ago

@guoguo12 You can review my proposal in PR #25 - it allows you to add language-specific search texts, but do not use universal ids, as I cannot found relevant ones.

guoguo12 commented 4 years ago

Cool, thanks. I'll take a look now.

guoguo12 commented 4 years ago

I uploaded the latest version to the Chrome Web Store, and it should be published soon. Thanks again for your PR!