iamdarkle / fancybox

Fancybox is a touch enabled and responsive tool for displaying images in a 'lightbox' that floats overtop of web page.
https://discuss.flarum.org/d/29102-fancybox
MIT License
6 stars 4 forks source link

Duplicated popups on image click #11

Closed luceos closed 2 weeks ago

luceos commented 1 month ago

Hi @iamdarkle,

We recently received reports of an issue with this extension. It seems that whenever you click an image it opens at least two fancybox__container entities, causing the problem that closing the preview with the "x" has to be done multiple times as well.

I'm investigating why this is happening, but I tested both 2.0 and the latest 1.x.

You can see it in action in this discussion: https://kagifeedback.org/d/3876

Your help would be appreciated.

iamdarkle commented 1 month ago

Hey, thanks for the report. I remember running into this same issue at some point (it seemed fixed since I couldn't reproduce it anymore). It used to happen only with standalone images, but I see it's also happening with the carousel (https://kagifeedback.org/d/3876-assets-are-not-loading-properly/24). Do you know if there's any extension that might be causing a conflict?

Autoimage from fof/formatting is expected not to work, but that shouldn’t be the case here, as it wouldn’t function at all. The forum is using the full image preview tags ([upl-image-preview url=])?

luceos commented 1 month ago

Yeah [upl-image-preview url=] is used.

Testing with fof formatting auto-image turned off produced the same, incorrect result.

I will try to reproduce this locally tomorrow.

MikeLundahl commented 2 weeks ago

Do you know if there's any extension that might be causing a conflict?

Hi there, I can add to this. After checking it out, I also found that fof/best-answer is causing this.

In my investigation, I found that commenting out the method post.pushAttributes({ isBestAnswer }) (line 63) in the file js/src/forum/addBestAnswerAction.js (fof/best-answer extension) makes the image issue go away for me.

iamdarkle commented 2 weeks ago

That's interesting, I'm concerned that another user who reported this with his php flarum info, doesn't seem to have fof/best-answer installed.

Flarum core: 1.8.5
PHP version: 8.3.8
MySQL version: 8.0.35-0ubuntu0.20.04.1
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, bz2, calendar, ctype, hash, fileinfo, filter, ftp, gettext, gmp, json, iconv, SPL, pcntl, session, standard, Phar, random, readline, Reflection, mbstring, shmop, SimpleXML, sockets, tokenizer, xml, mysqlnd, PDO, curl, dom, gd, intl, exif, mysqli, pdo_mysql, soap, xmlreader, xmlrpc, xmlwriter, zip, Zend OPcache
+-------------------------------------+---------+--------+
| Flarum Extensions                   |         |        |
+-------------------------------------+---------+--------+
| ID                                  | Version | Commit |
+-------------------------------------+---------+--------+
| flarum-likes                        | v1.8.0  |        |
| antoinefr-money                     | v1.3.1  |        |
| flarum-suspend                      | v1.8.1  |        |
| flarum-tags                         | v1.8.1  |        |
| flarum-flags                        | v1.8.0  |        |
| xypp-store                          | v1.1.5  |        |
| flarum-sticky                       | v1.8.0  |        |
| afrux-forum-widgets-core            | v0.1.7  |        |
| fof-user-directory                  | 1.3.3   |        |
| fof-byobu                           | 1.3.6   |        |
| flarum-bbcode                       | v1.8.0  |        |
| ziiven-money-transfer               | v0.2.5  |        |
| ziiven-decoration-store             | v1.1.2  |        |
| xypp-store-group                    | v1.0.2  |        |
| v17development-seo                  | v1.8.1  |        |
| the-turk-stickiest                  | 3.0.1   |        |
| nodeloc-split-view                  | 1.0.0   |        |
| neoncube-private-messages           | 1.3.0   |        |
| nearata-signup-confirm-password     | v3.1.1  |        |
| nearata-gif-avatars                 | 1.0.0   |        |
| migratetoflarum-canonical           | 1.0.0   |        |
| mickmelon-coloured-usernames        | 0.1.1   |        |
| litalino-more-bbcode                | 2.0.0   |        |
| justoverclock-welcomebox            | 2.0.2   |        |
| justoverclock-auto-post-count-badge | 1.0.0   |        |
| isaced-email-verification-switch    | 1.0.1   |        |
| ianm-boring-avatars                 | 1.0.0   |        |
| gtdxyz-badges                       | 1.0     |        |
| foskym-sorts-for-user-directory     | v0.3.2  |        |
| foskym-better-user-directory        | v0.1.2  |        |
| fof-user-bio                        | 1.4.0   |        |
| fof-upload                          | 1.5.5   |        |
| fof-socialprofile                   | 1.1.6   |        |
| fof-sitemap                         | 2.2.1   |        |
| fof-recaptcha                       | 1.3.4   |        |
| fof-reactions                       | 1.4.1   |        |
| fof-polls                           | 2.2.5   |        |
| fof-nightmode                       | 1.5.3   |        |
| fof-links                           | 1.2.3   |        |
| fof-linguist                        | 1.1.2   |        |
| fof-impersonate                     | 1.1.1   |        |
| fof-ignore-users                    | 1.2.1   |        |
| fof-forum-statistics-widget         | 1.2.1   |        |
| fof-formatting                      | 1.0.3   |        |
| fof-drafts                          | 1.2.11  |        |
| fof-disposable-emails               | 1.0.0   |        |
| fof-discussion-thumbnail            | 1.1.3   |        |
| fof-default-group                   | 1.1.2   |        |
| fof-bbcode-tabs                     | 1.0.3   |        |
| flarumite-simple-spoilers           | 1.0.0   |        |
| flarum-subscriptions                | v1.8.0  |        |
| flarum-statistics                   | v1.8.0  |        |
| flarum-pusher                       | v1.8.0  |        |
| flarum-nicknames                    | v1.8.0  |        |
| flarum-mentions                     | v1.8.3  |        |
| flarum-markdown                     | v1.8.0  |        |
| flarum-lock                         | v1.8.0  |        |
| flarum-lang-russian                 | 1.36.1  |        |
| flarum-lang-english                 | v1.8.0  |        |
| flarum-extension-manager            | v1.0.3  |        |
| flarum-emoji                        | v1.8.0  |        |
| davwheat-custom-sidenav-links       | 1.0.1   |        |
| datlechin-link-preview              | v1.5.0  |        |
| datlechin-copy-links                | v1.0.1  |        |
| darkle-fancybox                     | 2.0.0   |        |
| clarkwinkelmann-who-read            | 1.4.1   |        |
| clarkwinkelmann-roll-die            | 1.0.0   |        |
| clarkwinkelmann-money-to-all        | 1.0.0   |        |
| clarkwinkelmann-money-rewards       | 1.0.0   |        |
| clarkwinkelmann-group-list          | 1.0.0   |        |
| clarkwinkelmann-emojionearea        | 1.0.0   |        |
| clarkwinkelmann-comicsans           | 1.1.0   |        |
| blomstra-no-email-notifications     | 0.1.0   |        |
| blomstra-fontawesome                | 0.1.5   |        |
| askvortsov-moderator-warnings       | v0.6.3  |        |
| askvortsov-markdown-tables          | v1.2.1  |        |
| afrux-online-users-widget           | v0.1.6  |        |
| afrux-news-widget                   | v0.1.1  |        |
| acpl-mobile-tab                     | 1.4.4   |        |
+-------------------------------------+---------+--------+
Base URL: https://forumspace.ru
Installation path: /var/www/hidden/data/www/forumspace.ru
Queue driver: sync
Session driver: file
Scheduler status: Неактивно
Mail driver: smtp
Debug mode: off

So I guess it's related to the method... 🤔

MikeLundahl commented 2 weeks ago

I see... Would it be possible to share this composer file? I could make my instance match and see what more extensions are causing this. Perhaps confirm that it's the same method in the other extensions as well.

MikeLundahl commented 2 weeks ago

So I did a little experiment trying to debug this further. Not sure if this is the right path to take, but in: js/src/forum/index.js

in the CommentPost.prototype.initFancybox (line 20), I created a variable at the top that stops the following initialization from running multiple times with the following:

    if (this.fancyboxInitialized) return;
    this.fancyboxInitialized = true;

This seems to work for me. This might not solve the root cause, but it makes the popup image close on one click at least.

If you feel this solves the issue for now and is enough, I could create a PR.

luceos commented 2 weeks ago

@iamdarkle would this fix be suitable to you? Otherwise we'll apply this fix elsewhere.

MikeLundahl commented 2 weeks ago

@iamdarkle Just created a PR with the solution. Check it out and see if this is a good fix for you.

PR

iamdarkle commented 2 weeks ago

@luceos @MikeLundahl Yeah, it definitely does the trick. I think since the function is triggered by both oncreate and onupdate, it ends up initializing multiple times in some cases.

Thanks for the effort debugging this issue! 🙂

Tagged under 2.0.1.