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

fix capturing audio bug for yahoo dictionary. #14

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

alastor0325
Copy link
Contributor

The problem that we can't get the audio element is because Yahoo dictionary would generate audio elements later after we tried to get them.

Therefore, we could retry to get the element later, also add the maximum retry constraint in order to avoid infinite retry.

@johnyluyte
Copy link
Owner

johnyluyte commented Apr 16, 2018

I did ignore some online dictionaries because of the same reason that Yahoo dictionary currently behaves. Because keep retrying to get DOMs that may or may not exist seems like a hack to me. I believe there might be an Event-Driven or Async/Await Promise solution out there in the Chrome Extension API/architecture. But sadly I don't have time to dive into the documents.

Anyway, thanks for the fix. Really appreciate it. ❤️

p.s. Please make changes in your own branch(e.g. fix-yahoo-retry) instead of master, its much easier for the upstream owner to test your PR that way. Thanks! ❤️

@johnyluyte johnyluyte merged commit 0ebe6a5 into johnyluyte:master Apr 16, 2018
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