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

Detect attachEvent() support correctly, fixes #771 #772

Merged
merged 3 commits into from
Aug 16, 2017

Conversation

hallvors
Copy link
Contributor

No description provided.

@khanbot
Copy link

khanbot commented Jul 22, 2017

CLA signature looks good 👍

@kohler
Copy link
Collaborator

kohler commented Aug 2, 2017

Since main.js is only used in testing, I see no problem with this. It would be more self-consistent with the code above to write "attachEvent" in input, but it's fine either way.

@kohler
Copy link
Collaborator

kohler commented Aug 2, 2017

I'll wait a bit to merge just in case "attachEvent" in input arrives as a PR revision.

@edemaine
Copy link
Member

edemaine commented Aug 2, 2017

As discussed in #771, I'm not convinced this will be useful in isolation -- there's a call to attachEvent a few lines below that should be similarly protected, if this one is. But with a revision (and ideally testing to confirm this works where desired) it seems like a good idea.

@kohler
Copy link
Collaborator

kohler commented Aug 3, 2017

@edemaine This is a change to a file that is only used in testing KaTeX. I am happy to trust the submitter that the change was useful in his environment.

@kevinbarabash kevinbarabash merged commit a7351ff into KaTeX:master Aug 16, 2017
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.

5 participants