search

collective / collective.cover

A sane, working, editor-friendly way of creating front pages and other composite pages. Working now, for mere mortals.
48 stars 55 forks source link

Why do we have javascripts outside jsregistry.xml? #613

Open idgserpro opened 8 years ago

idgserpro commented 8 years ago

We have and installation that was using collective.cover 1.0a6. Trying to update it to 1.0a12, because we need https://github.com/collective/collective.cover/issues/526 in some IE11 tests for some clients, we realized the error wasn't fixed at all.

Even after running all upgradeSteps and re-running them again and again we were still getting errors. After lots of debugging and suffering, we found out that layout_edit.js isn't in jsregistry.xml: being in the template itself, this file was cached by the client browser and, since the fix was inside this js, it couldn't work since the cache is referencing an old file. After destroying the browser cache, it worked.

Any reason why we can't have the javascripts resources in jsregistry.xml? We believe this issue can haunt us in the future.

(We even think this may be related to this issue closed as invalid https://github.com/collective/collective.cover/issues/548)

hvelarde commented 8 years ago

again, just historical reasons: we need to do a huge refactor on all the JS code but it's going to take time and resources that we don't have right now; so, feel free to contribute.

regarding that issue you mentioned, I can't remember exactly why I closed it, but I think it was not related with this package.

idgserpro commented 8 years ago

No problem, at least we know it can be changed since it's not exactly a requirement.

We think it's related because we had the same error: since the tiles have the same uuid, when you're getting all attributes (like 'text', 'image') it gets the tile with uuid that doesn't have that attributes. The creation of uuids in js fixes it.

hvelarde commented 8 years ago

awesome! you're doing a great job on finding the root cause of many issues; thank you, guys!

idgserpro commented 8 years ago

These situation is found in:

https://github.com/collective/collective.cover/blob/0f4e20b7d61c6e9d9baf13ededb8bae53d5d5e56/src/collective/cover/browser/templates/compose.pt

https://github.com/collective/collective.cover/blob/ee61ed2542aaa886ec891fa8dc9c2b595bd6c1fe/src/collective/cover/browser/templates/layoutedit.pt

https://github.com/collective/collective.cover/blob/ee61ed2542aaa886ec891fa8dc9c2b595bd6c1fe/src/collective/cover/widgets/selectpreview_input.pt

https://github.com/collective/collective.cover/blob/985d0faafdd3b3401c2ba56d5c0c2d8aaac2b48e/src/collective/cover/tiles/carousel.py