Skip to content

[WebProfilerBundle] fix wrong variable for profiler counting ajax requests #26373

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

Closed
wants to merge 2 commits into from

Conversation

nicolaemarin
Copy link

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26364
License MIT
Doc PR symfony/symfony-docs#...

Symfony WebProfiler base js change variable tbodies to tbody for counting Ajax Requests.

@nicolas-grekas
Copy link
Member

ping @xkobal @Simperfit for double checking

@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Mar 2, 2018
@xkobal
Copy link
Contributor

xkobal commented Mar 2, 2018

That was tbodies before my fix but after reading another time the code, it seems that tbody is the correct object. @Simperfit must have the last word.

@Simperfit
Copy link
Contributor

Let me take this time to test this. But I think this is right too.

I'll test it today.

@nicolas-grekas nicolas-grekas changed the title fix wrong variable for profiler counting ajax requests [WebProfilerBundle] fix wrong variable for profiler counting ajax requests Mar 2, 2018
@n3o77
Copy link
Contributor

n3o77 commented Mar 2, 2018

@nicolas-grekas sorry for hijacking this PR. As i'm having trouble with the code affected in this PR it might make sense to coordinate this here before I create a PR.

@Simperfit your code caused some problems for me too. I'm loading a angular2 app which seems to make ajax requests before the tbodies.rows is defined ( error below ). I just fixed it with surrounding your code with an if to check for that.

Unfortunately I wasn't able to create a minimum code version to reproduce this. Maybe you have an idea under which circumstances exactly this could be the case.

Uncaught TypeError: Cannot read property 'count' of undefined
    at Object.renderAjaxRequests (order_now:267)
    at Sfjs.load.maxTries (order_now:267)
    at order_now:267
    at XMLHttpRequest.xhr.onreadystatechange (order_now:267)
    at XMLHttpRequest.wrapFn [as __zone_symbol___onreadystatechange] (vendor.bundle.js?v=1:5630)
    at ZoneDelegate.invokeTask (vendor.bundle.js?v=1:5063)
    at Zone.runTask (vendor.bundle.js?v=1:4871)
    at ZoneTask.invokeTask [as invoke] (vendor.bundle.js?v=1:5131)
    at invokeTask (vendor.bundle.js?v=1:5871)
    at XMLHttpRequest.globalZoneAwareCallback (vendor.bundle.js?v=1:5883)

(order_now is the pagename and references to the profiler code)

@nicolas-grekas
Copy link
Member

@n3o77 can you test the code in this PR and report if it fixes the issue for you?

@n3o77
Copy link
Contributor

n3o77 commented Mar 2, 2018

@nicolas-grekas yes, fixes the problem for me. Thank you @marinnicolae .

@n3o77
Copy link
Contributor

n3o77 commented Mar 2, 2018

Same problem is in v2.8

@tanasecosminromeo
Copy link
Contributor

@n3o77 Yep, problem is on v2.8 also - Fix works good. Thank you @marinnicolae

@nicolas-grekas
Copy link
Member

Thank you @marinnicolae.

nicolas-grekas added a commit that referenced this pull request Mar 2, 2018
…ting ajax requests (Marin Nicolae)

This PR was squashed before being merged into the 2.7 branch (closes #26373).

Discussion
----------

[WebProfilerBundle] fix wrong variable for profiler counting ajax requests

| Q             | A
| ------------- | ---
| Branch?       |  2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26364
| License       | MIT
| Doc PR        | symfony/symfony-docs#...

Symfony WebProfiler base js change variable tbodies to tbody for counting Ajax Requests.

Commits
-------

0fb83af [WebProfilerBundle] fix wrong variable for profiler counting ajax requests
@nicolas-grekas
Copy link
Member

Can anybody please check if the code on 3.4 works as expected?

@nicolaemarin
Copy link
Author

@nicolas-grekas Just tested on v3.4.5 and AjaxRequests does not appear cuz there are not appending in profiler bar. Will try to create a PR with the fix on 3.4 branch soon.

@nicolas-grekas
Copy link
Member

@marinnicolae please note that branch 3.4 already has some fixes, so might be just fixed already, you'll tell :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants