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

Errors fixes #2850

Closed
wants to merge 4 commits into from
Closed

Errors fixes #2850

wants to merge 4 commits into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 24, 2015

  • hide the error display properly when clearing out an error
  • allow clearing an error from the an error event handler

Triggers are synchronous so, if you are trying to disable the error in
an error handler, you end up throwing an error because the log line
refers to this.error_ which would then get cleared up when error(null)
is called.
@pam
Copy link

pam commented Nov 24, 2015

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: b2e67ce
Build details: https://travis-ci.org/pam/video.js/builds/93009983

(Please note that this is a fully automated comment.)

@gkatsev gkatsev added patch This PR can be added to a patch release. needs: LGTM Needs one or more additional approvals labels Nov 24, 2015
@misteroneill
Copy link
Member

LGTM.

We'll open a separate issue to trigger some kind of event when an error is removed/nullified.

@gkatsev
Copy link
Member Author

gkatsev commented Nov 24, 2015

Done #2851

@gkatsev gkatsev closed this in 5f90950 Nov 24, 2015
@gkatsev gkatsev deleted the errors branch November 24, 2015 20:38
@gkatsev gkatsev removed the needs: LGTM Needs one or more additional approvals label Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants