NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[WebProfilerBundle] Move ajax clear event listener initialization on loadToolbar#37077

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:4.4fromSeikyo:fix_ajax_event_listeners_stacking

Jun 2, 2020

Merged

[WebProfilerBundle] Move ajax clear event listener initialization on loadToolbar#37077

fabpot merged 1 commit intosymfony:4.4fromSeikyo:fix_ajax_event_listeners_stacking

Jun 2, 2020

Conversation

Copy link

Seikyo commentedJun 2, 2020•
edited
Loading

Q	A
Branch?	4.4
Bug fix?	no
New feature?	no
Deprecations?	no
Tickets	Fix#37073
License	MIT

Fix ajax clear event listener stacking on each ajax request made.
Comment says if should be synced withTwigBundle/Resources/views/base_js.html.twig

If not applicable feel free to deny.

carsonbot added Status: Needs Review WebProfilerBundle labels

Jun 2, 2020

Copy link

Member

stof commentedJun 2, 2020

This comment should be removed in the 5.0 branch of Symfony, as thebase_js file from TwigBundle got removed there.

Copy link

Member

stof commentedJun 2, 2020

I'm quite sure this bugfix should apply to older versions than master though.

Copy link

Author

Seikyo commentedJun 2, 2020

Sorry I'm a little lost in the process of proposing a pull request.
I checked the maintained versions from the roadmap and :

3.4 is not applicable as it does not have event listener on '.sf-toolbar-ajax-clear'
4.4 is applicable to this patch
5.0 and 5.1 are applicable to this patch but top comment should be removed as you suggested

Should I update the pull request branch to ">=4.4" ?

Copy link

Member

stof commentedJun 2, 2020

yeah, please submit this to 4.4 (and then check if the TwigBundle file needs the same patch).

Removing the top comment in 5.0 is not actually something related to your bugfix (it is related to what you wrote, though). This should probably be done as a separate PR against the branch.

Move ajax clear event listener initialization on loadToolbar

cf4587b

Seikyo force-pushed thefix_ajax_event_listeners_stacking branch from4edfe93 tocf4587bCompare

June 2, 2020 16:21

Seikyo requested a review fromdunglas as acode owner

June 2, 2020 16:21

Seikyo changed the base branch frommaster to4.4

June 2, 2020 16:25

Copy link

Author

Seikyo commentedJun 2, 2020

Pull request target was updated to origin/4.4 and rebased onto it.
No references to-clear were found in theTwigBundle/Resources/views/base_js.html.twig.