8000 symfony 2 profiler bar throws javascript error in IE8 · Issue #13447 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

symfony 2 profiler bar throws javascript error in IE8 #13447

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.

Sign up for GitHub

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

Jump to bottom
Closed
pjordaan opened this issue Jan 19, 2015 · 12 comments
Closed

symfony 2 profiler bar throws javascript error in IE8 #13447

pjordaan opened this issue Jan 19, 2015 · 12 comments
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) WebProfilerBundle

Comments

@pjordaan
Copy link

In IE8 and 9 the symfony 2 profiler throws a javascript error caused by the ajax requests logged.

IE8 and 9 do not have have a addEventListener object in XMLHttpRequest, but use attachEvent instead.

For IE8 and 9, in base_js.html.twig instead of this.addEventListener("readystatechange", function() {
it should be this.attachEvent("onreadystatechange", function () {
@stof
Copy link
Member
stof commented Jan 19, 2015

Can you send a pull request ?

@javiereguiluz
Copy link
Member

@pjordaan thanks for reporting this error and for taking the time to open this issue.

As @stof said, the best thing for us would be to have a pull request which we can easily merge on symfony's maintained branches. As you already have made PR to Symfony in the past, I guess you have no problems at all with this process. If not, please tell us and we'll look for other contributor to take over this issue. Thanks!

@fabpot fabpot added WebProfilerBundle Good first issue Ideal for your first contribution! (some Symfony experience may be required) labels Feb 5, 2015
@aik099
Copy link
aik099 commented Feb 10, 2015

I wonder, why jQuery wasn't used to attach event, which would have solved the problem once and for all.

@fabpot
Copy link
Member
fabpot commented Feb 10, 2015

@aik099 because we are not using jQuery... just plain JavaScript in the Web Profiler.

@aik099
Copy link
aik099 commented Feb 10, 2015

In fact I've found 2 more places, where addEventListener is used.

wvdweij pushed a commit to wvdweij/symfony that referenced this issue Feb 10, 2015
Fix profiler throw javascript errors on IE8 & IE9 symfony#13447
b79660d
wvdweij pushed a commit to wvdweij/symfony that referenced this issue Feb 10, 2015
Fix profiler throw javascript errors on IE8 & IE9 symfony#13447
3911135 
| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |
wvdweij pushed a commit to wvdweij/symfony that referenced this issue Feb 10, 2015
Fix profiler throw javascript errors on IE8 & IE9 symfony#13447
283868f 
| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |
@aik099 aik099 mentioned this issue Feb 10, 2015
Merged
@aik099
Copy link
aik099 commented Feb 10, 2015

@weaverryan , I'd recommend using my fix #13636 instead. Can you please test it for me?

@pyrech
Copy link
Contributor
pyrech commented Feb 10, 2015

Indeed, I think the changes made in #13636 are more complete than in #13634

@wvdweij
Copy link
wvdweij commented Feb 10, 2015

No problem, only when using attachEvent, the keyword this in the eventHandler refers to the document object instead of the caller object... So additional code should be written to handle the this context.

@aik099
Copy link
aik099 commented Feb 10, 2015

No problem, only when using attachEvent, the keyword this in the eventHandler refers to the document object instead of the caller object

I've solved this in aik099@04f3f43

fabpot added a commit that referenced this issue Feb 11, 2015
@fabpot
bug #13636 [WebProfilerBundle] Fixes event listener attaching error i…
7356876 
…n IE (aik099)

This PR was merged into the 2.6 branch.

Discussion
----------

[WebProfilerBundle] Fixes event listener attaching error in IE

I haven't tested the change, because I don't any working Symfony installation at hand. By the looks of changes it should work.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | not sure (do we have JS tests?)
| Fixed tickets | #13447
| License       | MIT
| Doc PR        |

Commits
-------

21693e4 [WebProfilerBundle] Fixes event listener attaching error in IE
@fabpot fabpot closed this as completed Feb 11, 2015
@parideo
Copy link
parideo commented Apr 2, 2015

Could you check the correctness of the fix of this issue? Seems to me that it broke the AJAX panel on Symfony 2.6.6.
If I replace base_js.html.twig and toolbar_js.html.twig with their corresponding files of Symfony 2.6.5 the AJAX panel works again.
All this verified using Firefox 36.0.4 on Windows 7 32bit.

@stof
Copy link
Member
stof commented Apr 2, 2015

@parideo this PR has been merged before 2.6.5. It is not a 2.6.6 change

@stof stof mentioned this issue Apr 2, 2015
Merged
@stof
Copy link
Member
stof commented Apr 2, 2015

I found the offending change though. See my PR

fabpot added a commit that referenced this issue Apr 2, 2015
@fabpot
bug #14170 Fix the AJAX profiling (stof)
ba84ac8 
This PR was merged into the 2.6 branch.

Discussion
----------

Fix the AJAX profiling

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | reported in #13447 (comment)
| License       | MIT
| Doc PR        | n/a

The fix for IE8 (#13978) which does not have the addEventListener method on XMLHttpRequest broke the feature for modern browsers because it was checking the existence on the wrong object. It is a method on the instance, not on the "class", and so should be checked on the prototype.

Commits
-------

9d6c0b1 Fix the AJAX profiling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @javiereguiluz @stof @aik099 @pyrech @pjordaan @wvdweij @parideo
0