Skip to content

Fix the toolbar JS for IE #13729

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

Merged
merged 1 commit into from
Feb 24, 2015
Merged

Fix the toolbar JS for IE #13729

merged 1 commit into from
Feb 24, 2015

Conversation

stof
Copy link
Member

@stof stof commented Feb 18, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR n/a
  • fix the binding of listeners on XMLHttpRequest to always use the addEventListener method. It does not make sense to change it to attachEvent as it is about binding listeners on DOM elements.
  • fix the feature detection for the event binding on DOM elements:
    • the attachEvent and addEventListener methods are on DOM elements, not on the document object
    • the standard method should be preferred in IE versions supporting both methods
  • avoid JS errors when XMLHttpRequest is not defined by avoiding to override its open method when the object is not there (old IE versions will still not intercept ajax calls though)

this is the complete fix for the code submitted in #13636 (the fix in #13684 was incomplete).

- fix the binding of listeners on XMLHttpRequest to always use the
  addEventListener method. It does not make sense to change it to
  attachEvent as it is about binding listeners on DOM elements.
- fix the feature detection for the event binding on DOM elements:
    - the attachEvent and addEventListener methods are on DOM elements,
      not on the document object
    - the standard method should be preferred in IE versions supporting
      both methods
- avoid JS errors when XMLHttpRequest is not defined by avoiding to
  override its open method when the object is not there (old IE versions
  will still not intercept ajax calls though)
@stof
Copy link
Member Author

stof commented Feb 18, 2015

For people reviewing this, most of the diff is just an indentation change in the JS code. Look at it with whitespace change ignore

@phansys
Copy link
Contributor

phansys commented Feb 19, 2015

This patch fixes the issue in FF and Chrome with readystatechange.
👍

@maximecolin
Copy link
Contributor

Works well in Chrome and Firefox. Thanks.

@fabpot
Copy link
Member

fabpot commented Feb 24, 2015

Thank you @stof.

@fabpot fabpot merged commit b7aa171 into symfony:2.6 Feb 24, 2015
fabpot added a commit that referenced this pull request Feb 24, 2015
This PR was merged into the 2.6 branch.

Discussion
----------

Fix the toolbar JS for IE

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | n/a

- fix the binding of listeners on XMLHttpRequest to always use the addEventListener method. It does not make sense to change it to attachEvent as it is about binding listeners on DOM elements.
- fix the feature detection for the event binding on DOM elements:
    - the attachEvent and addEventListener methods are on DOM elements, not on the document object
    - the standard method should be preferred in IE versions supporting both methods
- avoid JS errors when XMLHttpRequest is not defined by avoiding to override its open method when the object is not there (old IE versions will still not intercept ajax calls though)

this is the complete fix for the code submitted in #13636 (the fix in #13684 was incomplete).

Commits
-------

b7aa171 Fix the toolbar JS for IE
@stof stof deleted the fix_toolbar_js branch February 24, 2015 08:53
ostrolucky pushed a commit to ostrolucky/symfony that referenced this pull request Mar 25, 2018
This PR was merged into the 2.6 branch.

Discussion
----------

Fix the toolbar JS for IE

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | n/a

- fix the binding of listeners on XMLHttpRequest to always use the addEventListener method. It does not make sense to change it to attachEvent as it is about binding listeners on DOM elements.
- fix the feature detection for the event binding on DOM elements:
    - the attachEvent and addEventListener methods are on DOM elements, not on the document object
    - the standard method should be preferred in IE versions supporting both methods
- avoid JS errors when XMLHttpRequest is not defined by avoiding to override its open method when the object is not there (old IE versions will still not intercept ajax calls though)

this is the complete fix for the code submitted in symfony#13636 (the fix in symfony#13684 was incomplete).

Commits
-------

b7aa171 Fix the toolbar JS for IE
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.

4 participants