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

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

Closed
skarpik opened this issue Jun 9, 2024 · 14 comments
Assignees

Comments

@skarpik
Copy link

skarpik commented Jun 9, 2024

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

  • 2sxc version(s): 17.09.00 LTS and v17.10.00
  • Browser: all
  • DNN: 9.13.3
  • Hosting platform: IIS
  • Language: English
@iJungleboy
Copy link
Contributor

ATM I can't reproduce.

Our test still work as expected.

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

@skarpik
Copy link
Author

skarpik commented Jun 10, 2024

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
Copy link
Contributor

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
Copy link
Author

skarpik commented Jun 11, 2024

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
Copy link

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
Copy link
Contributor

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
Copy link
Author

skarpik commented Jun 23, 2024

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:

  • create empty DNN website (DNN v9.13.3)
  • install 2sxc v14.12.03 (https://github.com/2sic/2sxc/releases/tag/v14.12.03)
  • install 2shine v5.00.01 (the theme I developed was based upon this version of 2shine)
  • create a page with a 2sxc 50% text | 50% image content object (load all the default Apps and Content App)
  • test to make sure that everything works
  • upgrade to 2sxc v17.10.00
  • test once again (does Fancybox work?) - if not, gather any information for debugging

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
Copy link
Contributor

@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
Copy link
Author

skarpik commented Jun 26, 2024

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:

  • dnn v9.13.3
  • Microsoft CodeDom .NET Compiler (v3.6.0)
  • 2sxc version: v13.00.00 (released Dec 22, 2021)
  • 2sxc content: v13.00.00 (released Dec 22, 2021)
  • 2sxc theme: 2shine v05.00.01 (released Dec 20, 2021)

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

fixed it, will be in the next release.

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

@jeremy-farrance
Copy link

jeremy-farrance commented Jun 27, 2024 via email

@jeremy-farrance
Copy link

jeremy-farrance commented Jun 27, 2024 via email

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

No branches or pull requests

3 participants