Skip to content
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

[BUG] diff-dom freezing on CMS content refresh #7460

Closed
1 of 2 tasks
petrklus opened this issue Dec 20, 2022 · 13 comments
Closed
1 of 2 tasks

[BUG] diff-dom freezing on CMS content refresh #7460

petrklus opened this issue Dec 20, 2022 · 13 comments

Comments

@petrklus
Copy link

petrklus commented Dec 20, 2022

Description

We are experiencing issues with the CMS refresh. For certail plugin configurations and post-plugin modification, the browser tab first freezes, later to crash due to out of memory error. We have confirmed the same on Google Chrome / Mozilla Firefox.

Steps to reproduce

  1. Trigger content/structure board refresh
  2. Tab crashes

Expected behaviour

The page content to update.

Actual behaviour

The diff between incoming document and actual document freezes up:

https://github.com/django-cms/django-cms/blob/develop/cms/static/cms/js/modules/cms.structureboard.js#L1291

Only tab restart fixes the issue.

Screenshots

image

Additional information (CMS/Python/Django versions)

Django CMS 3.11.0 or 1, Django 3.2.

Tested on Mozilla Firefox and Google Chrome

Do you want to help fix this issue?

We have looked into this further and the issue seems to be down to DiffDOM version 2.5.1. We have tried upgrading to the latest version 4.2.8, which works in debug mode, however, is not compatible with the current UglifyJS config.

  • Yes, I want to help fix this issue and I will join #workgroup-pr-review on Slack to confirm with the community that a PR is welcome.
  • No, I only want to report the issue.
@Rogell
Copy link

Rogell commented Mar 26, 2023

This issue happened to me also and I can confirm that upgrading to the latest DiffDOM (version 5.0.4) fixes the problem. Works with debug mode on and off. UglifyJS doesn't throw any warnings regarding DiffDOM.
I will not create a pull request as I don't have time to read all contribution requirements so if anyone wants to fix it here are my changes.

Details:
Branch: release/3.11.x
Python version: 3.11
NPM version: 6.14.4
System: Ubuntu 20.04.2 LTS on WSL2
Django: 3.2

Changes:
package.json:52 -> "diff-dom": "5.0.4",
cms/static/cms/js/modules/cms.structureboard.js:11 -> import { DiffDOM } from 'diff-dom';

@stale
Copy link

stale bot commented Jun 24, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 24, 2023
@petrklus
Copy link
Author

@fsbraun would you know of anyone with the CMS editor code experience who can look into this?

@stale stale bot removed the stale label Jun 24, 2023
@marksweb
Copy link
Member

@petrklus whats happening with the plugins that might give us a clue on how to reproduce this?

@fsbraun
Copy link
Sponsor Member

fsbraun commented Jun 24, 2023

@petrklus Can you reopen #7461 ? So far, I have not been able to make the tests pass with the updated diffdom dependency. OK, I just realized, I could reopen it myself.

@fsbraun
Copy link
Sponsor Member

fsbraun commented Jun 26, 2023

@petrklus I think I have a solution (#7599) for v4:

  • ChromeHeadless (using puppeteer) instead of PhantomJS
  • (What kept me from succeeding at first were insufficient viewport dimensions in ChromeHeadless.)
  • Update of karma

Do you think you can transfer the changes of #7599 to #7461 to see if we get tests running?

vinitkumar added a commit to SocialSchools/django-cms that referenced this issue Jun 27, 2023
Code borrowed from:
django-cms#7599
Also, based on on the ticket and work of django-cms#7460

Co-Authored-By: Petr Klus <petr@klus.co.uk>
Co-Authored-By: Fabian Braun <fsbraun@gmx.de>
fsbraun added a commit that referenced this issue Jul 3, 2023
* fix: add updates same as PR: #7599

Code borrowed from:
#7599
Also, based on on the ticket and work of #7460

Co-Authored-By: Petr Klus <petr@klus.co.uk>
Co-Authored-By: Fabian Braun <fsbraun@gmx.de>

* fix: remove plugin that has been removed from the deps

* fix: polyfill promise to fix lint errors and also support promise in browsers like IE11 and Opera mini

* feat: update to nodejs 18 LTS to see if it builds

* feat: update autoprefixer

* fix: upgrade autoprefixer

* fix: no polyfill only support major browsers with major market share:

* fix: upgrade to nodejs 18 in .nvmrc as well

---------

Co-authored-by: Petr Klus <petr@klus.co.uk>
Co-authored-by: Fabian Braun <fsbraun@gmx.de>
@DmytroLitvinov
Copy link

DmytroLitvinov commented Sep 8, 2023

Hey @fsbraun ,
We struggled in our project with the same issue:

  • DjangoCMS 3.11.3
  • Django 3.2

After swapping to develop version in our staging, we see there is no issue.
(In develop branch there is such commit d8e9c52

Helpful links:

Do you think we can release new minor version of DjangoCMS like 3.11.4 with such fix? I am ready to provide help here with a contribution and I am really would be glad to do that.

@fsbraun
Copy link
Sponsor Member

fsbraun commented Sep 8, 2023

I am literally working on it at the moment :-)

@fsbraun
Copy link
Sponsor Member

fsbraun commented Sep 8, 2023

@DmytroLitvinov Out now!

@DmytroLitvinov
Copy link

Wow. Thank you 🤩
Going to deploy it to staging and production new minor version.

@fsbraun
Copy link
Sponsor Member

fsbraun commented Sep 8, 2023

One after the other!

Copy link

stale bot commented Dec 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 7, 2023
@fsbraun
Copy link
Sponsor Member

fsbraun commented Dec 7, 2023

Fixed in django CMS version 3.11.4

@fsbraun fsbraun closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done/merged
Development

No branches or pull requests

5 participants