2sic / 2sxc

DNN + 2sxc = #DNNCMS - This tool helps web designers and developers prepare great looking content in DNN (DotNetNuke). It's like mixing DNN with Umbraco and Drupal :)
http://2sxc.org
MIT License
141 stars 38 forks source link

Upgrading to 2sxc v17.09.00 breaks lightbox functionality in website with v13.00.00 Content App #3424

Closed skarpik closed 3 weeks ago

skarpik commented 1 month ago

I'm submitting a

[x] bug report => search github for a similar issue before submitting

...about

[x] Content Types or data management [x] other / unknown

Current Behavior / Expected Behavior

I upgraded a DNN website that was originally built with 2sxc v13.0.0 and Content App v13.00.00 to 2sxc v17.09.00 LTS. The website had successfully been upgraded to 2sxc v14.12.03 LTS and then later to 2sxc v16.07.00 LTS. However, upgrading to 2sxc v17.09.00 LTS breaks the lightbox functionality in basic content apps (for instance, image 50% | text 50%). Lightbox functionality is also broken the photo gallery app (Gallery App v7 using Fancybox3).

If an image is clicked on, instead of a lightbox appearing, the image is shown directly as a JPEG.

Websites running older versions of 2sxc should be upgraded to 2sxc v17.09.00 or newer for performance and security. However, this bug breaks websites upon upgrade (however, probably only for older versions of the Content App).

To see the issue in action, visit the website:

https://basilthetortoise.com/

and click on the photo of the tortoise. The same behaviour can be seen in the Photo Gallery page:

https://basilthetortoise.com/Photo-Gallery

There is no custom 2sxc code. Everything is directly "out-of-the-box" unmodified.

Instructions to Reproduce the Problem

Proper functioning:

Screenshot 2024-06-09 165906


Screenshot 2024-06-09 165933

Broken by upgrade:

Screenshot 2024-06-09 170018

If it is helpful, I can send a backup of the 2sxc v16.07.00 website to someone to test. This version works perfectly. Upgrading it to v17.09.00 breaks it.

Your environment

iJungleboy commented 1 month ago

ATM I can't reproduce.

Our test still work as expected.

pls check your JS console and see if you see other errors

skarpik commented 1 month ago

I decided to simplify things. In case there was something wrong with my custom theme, I created a new page with your 2shine BS5 theme:

https://basilthetortoise.com/Test

It is very simple - just a single Content App object (a text 50% | image 50% item).

Looking again at the JS Console, I realized that I hadn't waited long enough before clicking on the photo and that is why I missed the error. I have found an error which shows up a bit delayed as a failed promise. I have attached the contents of the Console from Firefox and taken a screen shot of the Console from Edge.

You might find it more convenient to look at the console directly yourself (especially to inspect the content of objects).

The website is DNN v9.13.03 and 2sxc v17.10.0. Locally I have a 2sxc v17.09.00 version (which displays the same behaviour) and the pre-upgrade v16.07.00 version which works perfectly.

console-export-2024-6-10_10-37-50.txt

Screenshot 2024-06-10 103045

iJungleboy commented 1 month ago

So obviously in your case it can't load the fancybox.js from the CDN.

Can you check the network tag? is your browser "offline"?

skarpik commented 1 month ago

Here are the network traces for the v16.07.00 LTS and 17.09.00 LTS versions. Fancybox loads for v16 but not for v17. I have found no errors to indicate why. I don't think it is a browser problem - the problem is there with Edge, Firefox and Chrome on my workstation. I've also tested it on my cellphone (5G connection not WiFi) and get the same result.

2sxc v16.07.00

2sxc v16 07 00

2sxc v17.09.00

2sxc v17 09 00
jeremy-farrance commented 1 month ago

I haven't been able to spot the cause of this but I can verify its happening (broken) as Steve describes. I've been doing a few upgrades a day and will watch for this and see if I can find an example and take a closer look.

Also seeing the same delayed console error (failed promise).

Same behavior in MS Edge.

Only thing I notice that I don't see mentioned above is that the page source does NOT contain the line to load fancybox at all. So I think the thing to look at is the old code that put that in the page source and see if there is a compatibility issue with the old ways 2sxc was adding or post-load processing assets. Maybe the JS handling the data-optimizations is missing from the JS core or other client side elements?

Sorry for guessing wildly, I don't have time to dig deeper at the moment.

I just wanted to verify it was a problem and hoping to see the problem identified so a fix gets into an upcoming release. This is definitely going to affect a lot of our sites as we get through more upgrades!

iJungleboy commented 4 weeks ago

I still don't have a clue here.

Maybe you could review the insights before/after and see if it's even trying to add it (eg. does fancybox occur in insights) or not at all...

skarpik commented 3 weeks ago

Let's see what I can do to bring some light to this issue. This website was created almost three years ago and possibly something that I did its creation makes an outlier. So to make sure that I am not sending you (and anyone else) on a wild goose chase. So I want to create a clean 2sxc test website with 2sxc 14.12.03 LTS and 2sxc Content 13.00.00.

Here's what I am planning:

When I install 2sxc v14.12.03, will it automatically add the v13.00.00 content apps, or do I need to do something special?

iJungleboy commented 3 weeks ago

@skarpik
Yes, older versions of 2sxc automatically recommend older templates.

I believe the install dialog will also show you some hint or a direct link to the one it will install, so you could verify it.

alternatively you could also manually install and old template from https://github.com/2sic/2sxc-content-app/releases

skarpik commented 3 weeks ago

I looked back into my notes and found that the website in question was built on Dec. 24, 2021 with v13.00.00 and 2sxc Content v13.0.0. So to I created the website:

https://webdev.stevekarpik.com/

It has only one page and was built using the following:

After upgrading to 2sxc v17.10.00, the lightbox no longer works (clicking on the panda photo takes you to a page with just the JPEG).

The console from Chrome gives more information than Edge or Firefox but I don't know what to make of it:

Screenshot 2024-06-26 185517

The images below show the website pre- and post-upgrade.

Pre-Upgrade

Screenshot 2024-06-26 092219

Screenshot 2024-06-26 092301

Screenshot 2024-06-26 092451

Screenshot 2024-06-26 092545

Post-Upgrade

Screenshot 2024-06-26 092700

Screenshot 2024-06-26 092751

iJungleboy commented 3 weeks ago

@skarpik thanks for this.

I also checked the network tab and it's not loading fancybox.js

My first findings are that these apps use an older "name" of the IPageService. It should of course still work, but here's my analysis and current workaround.

  1. A long time ago we started introducing services and they were kind of "spread all over the place" in terms of the namespaces. So the first IPageService was in ToSic.Sxc.Web.IPageService.
  2. With time, we changed our strategy to put all of them in ToSic.Sxc.Services so they are easier to find in the docs - especially if you needed to know if other services existed that could be usefuly
  3. So the latest IPageService is on ToSic.Sxc.Services.IPageService
  4. Internally they are the same page service, just an older synonym for the same thing

Now interestingly something seems off when the old name is used. This doesn't make sense and needs fixing, but if you change the `/bs?/shared/assets.cshtml from:

@inherits Custom.Hybrid.Razor12
@{
  // Basics / Preparation
var pageCss = GetService<Connect.Koi.ICss>();         // Service to get CSS information about the current Theme
  var page = GetService<ToSic.Sxc.Web.IPageService>();  // Service to set titles etc. on the page

to

@inherits Custom.Hybrid.Razor12
@{
  // Basics / Preparation
var pageCss = GetService<Connect.Koi.ICss>();         // Service to get CSS information about the current Theme
  var page = GetService<ToSic.Sxc.Services.IPageService>();  // Service to set titles etc. on the page

...everything seems to fix itself.

So that's the quick-fix, I'll try to figure out what the root cause is and of course fix this in the next release.

iJungleboy commented 3 weeks ago

Further analysis: I thought the implementations are identical for both old/new IPageService. It turns out that the old one has a wrapper-implementation (calling the new one). This seems to not forward the context (which knows what App it is etc.) to the inner implementation, which caused the problem.

What confuses me is that I would think this should have appeared in earlier versions as well, but maybe nobody updated old systems before LTS or something.

iJungleboy commented 3 weeks ago

fixed it, will be in the next release.

Thanks for reporting and @skarpik special thanks for creating a test-setup!

jeremy-farrance commented 3 weeks ago

[celebrate] Jeremy Farrance reacted to your message:


From: iJungleboy @.> Sent: Thursday, June 27, 2024 6:06:01 AM To: 2sic/2sxc @.> Cc: Jeremy Farrance @.>; Comment @.> Subject: Re: [2sic/2sxc] Upgrading to 2sxc v17.09.00 breaks lightbox functionality in website with v13.00.00 Content App (Issue #3424)

fixed it, will be in the next release.

Thanks for reporting and @skarpikhttps://github.com/skarpik special thanks for creating a test-setup!

— Reply to this email directly, view it on GitHubhttps://github.com/2sic/2sxc/issues/3424#issuecomment-2193872894, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAE7P5J3RWFXJPZJNLIW45TZJOTUTAVCNFSM6AAAAABJBGDISGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJTHA3TEOBZGQ. You are receiving this because you commented.Message ID: @.***>

jeremy-farrance commented 3 weeks ago

[like] Jeremy Farrance reacted to your message:


From: iJungleboy @.> Sent: Thursday, June 27, 2024 6:06:01 AM To: 2sic/2sxc @.> Cc: Jeremy Farrance @.>; Comment @.> Subject: Re: [2sic/2sxc] Upgrading to 2sxc v17.09.00 breaks lightbox functionality in website with v13.00.00 Content App (Issue #3424)

fixed it, will be in the next release.

Thanks for reporting and @skarpikhttps://github.com/skarpik special thanks for creating a test-setup!

— Reply to this email directly, view it on GitHubhttps://github.com/2sic/2sxc/issues/3424#issuecomment-2193872894, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAE7P5J3RWFXJPZJNLIW45TZJOTUTAVCNFSM6AAAAABJBGDISGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJTHA3TEOBZGQ. You are receiving this because you commented.Message ID: @.***>