Skip to content

Fixes event listener attaching error #13684

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 16, 2015
Merged

Conversation

bartru
Copy link

@bartru bartru commented Feb 13, 2015

Q A
Bug fix? [yes]
New feature? [no]
BC breaks? [no]
Deprecations? [no]
Tests pass? [yes]
Fixed tickets [none]
License MIT
Doc PR [none]

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | [none]
| License       | MIT
| Doc PR        | [none]
@fabpot
Copy link
Member

fabpot commented Feb 16, 2015

Thank you @bartru.

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

Discussion
----------

Fixes event listener attaching error

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | [none]
| License       | MIT
| Doc PR        | [none]

Commits
-------

34b489d Fixes event listener attaching error
@stof stof mentioned this pull request Feb 18, 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
fabpot added a commit to symfony/web-profiler-bundle 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 symfony/symfony#13684 was incomplete).

Commits
-------

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

Fixes event listener attaching error

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | [none]
| License       | MIT
| Doc PR        | [none]

Commits
-------

34b489d Fixes event listener attaching error
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