cubecart / v6

CubeCart Version 6
https://cubecart.com
72 stars 59 forks source link

Edit Product - Slow in Firefox #3102

Closed abrookbanks closed 1 year ago

abrookbanks commented 1 year ago

This seems to have something to do with jQuery.

Chrome: 1.6 seconds Firefox: 7 seconds

bhsmither commented 1 year ago

Any difference between grabbing jQuery from a specific CDN, vs. being on a web server located on a local network?

bhsmither commented 1 year ago

(Probably of very little help, but...) My Firefox connecting to a local web server fetching page resources: From CubeCart's debug listing -- Page Load Times: POST = 0.1785 seconds GET = 0.2889 seconds

From Firefox's Network Analysis: 2.18 seconds total The largest delays: Blocked by uMatrix against Zopim and agent.cubecart.com - caused 300ms delay before fetching next item AJAX requests for list of files for group=1 and group=2 (concurrently) - 400ms after previous fetch, wait time of 75/122ms

abrookbanks commented 1 year ago

I think it will be down to how FF interprets the JS. It must be a common issue ... Probably resolved in newer jQuery versions.

hairy-dog commented 1 year ago

I recently resolved several issues by uninstalling Firefox. I like their ideas, but I'm less keen on their browser

abrookbanks commented 1 year ago

CDN seems to make no difference. It has to be a JS engine issue. I expect it's something to do with selecting DOM elements.

abrookbanks commented 1 year ago
Screenshot 2022-12-28 at 08 57 52
abrookbanks commented 1 year ago

These lines from admin.js

s > r && $("#page_content").css('min-height',s + 100 +'px'), $('input[type="checkbox"]').each(function() {
        $(this).parent().hasClass("custom-checkbox") || $(this).wrap("<div class='custom-checkbox'></div>"), $(this).is(":checked") ? $(this).parent().addClass("selected") : $(this).parent().removeClass("selected")
abrookbanks commented 1 year ago

So I'm convinced the issue concerns the jQuery each loop when selecting input variables.

The Flame Graph complains about both:

admin.js:897 $('input[type="checkbox"]').each(function() {

admin.js:326 $(":input, :input:hidden").each(function() {

abrookbanks commented 1 year ago

Removing this block from admin.js significantly improves load time:

$(":input, :input:hidden").each(function() {
            $(this).hasClass("original-fix") || $(this).attr("original", $(this).val())
        }).change(function() {
            pageChanged(this)
        })
abrookbanks commented 1 year ago

So maybe the solution is to remove that code and think about how to notify the user of changes before navigating away.

abrookbanks commented 1 year ago

Pleased with this.

bhsmither commented 1 year ago

Can we know how many files were being listed in the FileTree?

abrookbanks commented 1 year ago

Can we know how many files were being listed in the FileTree?

4003 (to be precise)

bhsmither commented 1 year ago

I FTP'd 4000 images to a sub-folder in /images/source/. I then viewed in admin, FileManager, Images, and clicked on that sub-folder. No images showed - as expected - because the list here is simply a database query.

I then clicked on Update File List for that sub-directory. I got a Gateway Timeout from the web server (approx 60 seconds), followed by an indication in PHP's error log of a Fatal Error that 'max_execution_time' had been exceeded (60 seconds plus misc delays) with having generated 3166 rendered images. Having the browser resend the querystring finished that task.

BUT, the browser was taking too long to render the resulting page - showing very little of a standard admin page. I closed the browser tab and reloaded the FileManager, Images screen.

When I clicked the sub-folder icon, it took about 10 minutes to render what CubeCart's debug data shows what took 13 seconds to generate - about 5 minutes of nothing, then about 5 minutes of the 'loading_content' spinner.

So, my initial conclusion is to enforce (or make urgent suggestions on the Warning banner) a limit on the number of images per folder.

  1. I will test removing that $(":input, :input:hidden") statement.

  2. I will also test a variation alluded to in:

    http://api.jquery.com/input-selector/

    which says: "To achieve the best performance when using :input to select elements, first select the elements using a pure CSS selector, then use .filter(":input")."

(Using jQuery 1.12.4)

bhsmither commented 1 year ago

As for test (1), 7 seconds - with a spinner - from DOMContentLoaded to Done.

As for test (2), 10 seconds - but no spinner, using the code:

From near line 326:
}), $(":input, :input:hidden").each(function() {
To:
}), $("input, textarea, select, button").each(function() {

It did find hidden elements. I did not need to add the .filter() function.

bhsmither commented 1 year ago

It's very much faster when the profiler is not running.

abrookbanks commented 1 year ago

Hi Brian. Uploading files then using the update file list tool is something totally different. It's related but not relevant.

The code changes commit literally changed the page load time for this store from around seven seconds to near instant. Both myself and merchant reproduced that.

This was with latest Firefox. It's an established store that already had an up to date image database.

abrookbanks commented 1 year ago

It's very much faster when the profiler is not running

I didn't experience that. It was the same slow speed with or without the profiling tool.

abrookbanks commented 1 year ago

I think also the "pageChanged" system in CubeCart is very crude. The new jQuery lib is much more reliable and even checks on events such as page refresh as well as navigating away.

Source: https://stackoverflow.com/questions/7317273/warn-user-before-leaving-web-page-with-unsaved-changes