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

Les Echos : Enhance error handling (querySelector). Refactor MutationObservers. #243

Merged
merged 4 commits into from
Sep 1, 2024

Conversation

Write
Copy link
Collaborator

@Write Write commented Sep 1, 2024

This is a complete refactor that enhance robustness for LesEchos.
Tested on Firefox / Chrome. Race condition where two buttons were added seems fixed too.

  • Reduce querySelector usage by passing metaElement to isPremium function.
  • Fix race condition.
  • Correctly disconnect observer.

- Reduce querySelector usage by passing metaElement to isPremium function.
- Fix race condition.
- Correctly disconnect observer.
…ck to callbackDirectAccess to differentiate the two callbackes. Removing useless check in isPremium function
@Altonss
Copy link
Contributor

Altonss commented Sep 1, 2024

This is a complete refactor that enhance robustness for LesEchos. Tested on Firefox / Chrome. Race condition where two buttons were added seems fixed too.

* Reduce querySelector usage by passing metaElement to isPremium function.

* Fix race condition.

* Correctly disconnect observer.

Issues doesn't seem fixed on firefox (tried latest commit of the PR):

  • the double button still happens when navigating using page to page
  • button sometimes doesn't appear when opening single article individually (not related to NoScript!)

@Write
Copy link
Collaborator Author

Write commented Sep 1, 2024

Je confirme pour le double boutons... par contre cela ne m'arrive que la première fois (au démarrage de l'extension).

Je n'ai pas réussi à reproduire le problème du bouton qui ne s'affiche pas du tout en ouvrant un article seul.

@Write
Copy link
Collaborator Author

Write commented Sep 1, 2024

J'ai pris le taureau par les cornes. Normalement, c'est bon pour le double bouton.
Pour l'autre bug, je n'arrive pas à reproduire.

@lovasoa lovasoa merged commit 04feb4a into lovasoa:master Sep 1, 2024
1 check passed
@Write
Copy link
Collaborator Author

Write commented Sep 1, 2024

Maintenant sur Firefox il y a tout le temps deux fois le bouton.
Sur Chrome c'est ok, mais très rarement le bouton n'apparait en effet pas du tout.

Je suis vraiment désolé,

Je proposerai un nouveau PR quand j'aurai vraiment testé TOUS les cas de figures sur TOUS les navigateurs de fond en comble.

Ok, je n'ai rien dit, c'est juste que j'avais l'userscript qui tournait en même temps sur Firefox.

Le double bouton devrait être ok, cependant sur Chrome de temps à autre en effet le bouton n'apparaît pas. Peux être sur Firefox aussi, mais pas réussi à reproduire encore.

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.

3 participants