1907272 - XHR request not firing until page is refreshed or dev tools are open

Status: RESOLVED FIXED

(Core :: DOM: Networking, defect, P2)

Description

[adam]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36 Edg/126.0.0.0

Steps to reproduce:

Visit web page and expect prices to load via XHR:
https://www.alocalprinter.co.uk/thread-stitched-notebooks/

Issue does not occur in Chrome, Edge, Opera or Safari.

Actual results:

XHR was not fired and prices did not load. Following reported in console:

Uncaught TypeError: elem is undefined
jQuery 35
<anonymous> https://www.alocalprinter.co.uk/thread-stitched-notebooks/:1642
jQuery 2
jquery.js:8143:9

Refresh page and loads fine with no errors and prices load.

This appears to be a timing/caching issue. Refresh the page and prices load fine. Visit other products while dev tools is open and the issue does not occur. Visit other products with dev tools closed and it might or might not occur. If it does, open dev tools and the same error shows. Refresh and it goes away.

Expected results:

XHR request should fire consistently to load prices.

If the above error is genuine, why does it only occur intermittently and goes when the page is refreshed or dev tools are open with caching disabled?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Networking' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Sunil Mayya]

Thanks adam for reporting this.
I can reproduce this issue locally.
We will check this.

Comment 3

[Sunil Mayya]

I need to check and confirm if this is an issue due to Necko code.

Comment 4

[adam]

We've only had this reported recently (last few weeks). Present in 127 and 128 so may have been introduced in 127.

Comment 5

[Thomas Wisniewski [:twisniewski]]

This doesn't seem to be something new to me. I just ran mozregression on my macbook, and as far back as Firefox 80 the prices don't consistently load for me on each page-load/refresh, with the same "elem" error.

Comment 6

[Thomas Wisniewski [:twisniewski]]

So this is the "elem" error:

Uncaught TypeError: can't access property "getAttribute", elem is undefined

And this is the related code; the error is thrown on the first line, Tablesaw.init():

    $(document).ready(function() {
        Tablesaw.init();
        pricesScrollable();
        $('body').on('change', '#firstOption', function(){
            selectAttrVal = $(this).val();
            ajaxUpdate();
        });
        $('body').on('change', '.customOptions', function(){ 
            ajaxUpdate(); 
        });
        $('body').on('click', 'a.add-to-cart', function(e){ 
            e.preventDefault(); 
            addToCart($(this).data('carturl')); 
        });
        $(window).resize( function() {
            $("#product-summary").css('height', 'auto'); 
            $("#product-summary-left").css('height','auto');
            $("#product-summary-img").css('height','auto');
            alignBoxes();
            pricesScrollable();
        });
            
        ajaxUpdate();
        alignBoxes();
    });

But the actual XHR for the prices doesn't seem to be made until the ajaxUpdate() call, so it makes sense that it's never sent, as the error prevents things from getting that far.

As such I think the key will be to figure out what is causing the "elem" error, and it might not really be a bug with the XHRs themselves.

Comment 7

[adam]

Agreed. I've tried modyfing Tablesaw.init(); to Tablesaw.init('#price-grid'); which it probably should be. Slightly different error now but still an error causes the script to fail and prices not load. This is Magento (upgraded to 2.4.7 recently) and it looks like there's been a change to script handling. This was inline script but now it has been auto-extracted, compressed and moved to the end. That may now be what's causing the issue - but it's only in Firefox.

This script is fired on document ready so everything should be there, but the error is suggesting it's not. This is what Tablesaw.init() does:

init: function(element) {
	// Account for Tablesaw being loaded either before or after the DOMContentLoaded event is fired.
	domContentLoadedTriggered =
		domContentLoadedTriggered || /complete|loaded/.test(document.readyState);
	if (!domContentLoadedTriggered) {
		if ("addEventListener" in document) {
			// Use raw DOMContentLoaded instead of shoestring (may have issues in Android 2.3, exhibited by stack table)
			document.addEventListener("DOMContentLoaded", function() {
				Tablesaw._init(element);
			});
		}
	} else {
		Tablesaw._init(element);
	}
}

The issue occurs when domContentLoadedTriggered is true. Then Tablesaw._init(element) fails.

No idea what /complete|loaded/.test(document.readyState) does - Tablesaw is a 3rd party library we use.

Comment 8

[Thomas Wisniewski [:twisniewski]]

@adam, yeah, it's actually a race condition with the site's scripts, and I'd be shocked if it doesn't affect other browsers as well.

I'll spare the gory details of my hunt to figure this one out, but the short version is that you need to initialize Tablesaw after ajaxUpdate fetches and replace the table with the one Tablesaw needs. Otherwise, it will not see the elements it needs, because the markup isn't there yet, throw the error, and ajaxUpdate will not even run.

That's purely by chance, because the code does this:

$(document).ready(function() {        
    Tablesaw.init();
    ajaxUpdate();
});

Yet Tablesaw.init() does this:

  init: function(element) {
    // Account for Tablesaw being loaded either before or after the DOMContentLoaded event is fired.
    domContentLoadedTriggered =
      domContentLoadedTriggered || /complete|loaded/.test(document.readyState);
    if (!domContentLoadedTriggered) {
      if ("addEventListener" in document) {
        // Use raw DOMContentLoaded instead of shoestring (may have issues in Android 2.3, exhibited by stack table)
        document.addEventListener("DOMContentLoaded", function() {
          Tablesaw._init(element);
        });
      }
    } else {
      Tablesaw._init(element);
    }
  }

So if the document's readyState already happens to allow it, Tablesaw will try to initialize right away, throw the elem undefined error, and ajaxUpdate will never get a chance to run (hence the XHR never happens).

So if you drop the current Tablesaw.init call, and instead call it after the ajaxUpdate XHR finishes, things should work consistently. They do for me if I alter ajaxUpdate to do this, for instance:

            if (response !== ''){
                $('#price-grid tbody').html(response.result);
                if (!window.tableSawInitialized) {
                  Tablesaw.init();
                  window.tableSawInitialized = true;
                } else {
                  $('#price-grid').tablesaw().data("tablesaw").refresh();
                }
            }

There's likely a more pleasant fix you can use instead, but you know your code better than I do :)

Comment 9

[adam]

@thomas, many thanks for your detailed investigation into this. I have modifed our script as suggested and it does address the issue. However...

While this is easily reproducible in Firefox, it does not occur in any other browser. We've had the same code running for 9 years now (albeit Magento 1 and 2) and never had this occur before. It has only started in the last few weeks and only with Firefox. We upgraded to Magento 2.4.7 recently.

So if the document's readyState already happens to be loading, Tablesaw will try to initialize right away.

While the Tablesaw.init() function handles being called before the document is ready, we're actually calling it on $document.ready(). At that point readystate should not still be loading; surely it should be loaded or complete, no? The price table always exists - the ajax call just updates the table body (prices).

To me it looks like there is an issue in Firefox - namely jQuery $document.ready() can fire before the document has finished loading. Maybe this is not down to a Firefox update but has come to light due to changes in latest Magento that moves inline script to the end of the document. The script is inline in the page template but is now being extracted and moved by Magento, probably as part of performance changes. This shouldn't be an issue.

Comment 10

[Thomas Wisniewski [:twisniewski]]

Ah, sorry, I should have been clearer about that. jQuery doesn't just wait for DOMContentLoaded, it does some complicated stuff before calling your code in ready:

// Catch cases where $(document).ready() is called
// after the browser event has already occurred.
// Support: IE <=9 - 10 only
// Older IE sometimes signals "interactive" too soon
    if ( document.readyState === "complete" ||
        ( document.readyState !== "loading" && !document.documentElement.doScroll ) ) {

        // Handle it asynchronously to allow scripts the opportunity to delay ready
        window.setTimeout( jQuery.ready );

    } else {

        // Use the handy event callback
        document.addEventListener( "DOMContentLoaded", completed );

        // A fallback to window.onload, that will always work
        window.addEventListener( "load", completed );
    }

So sometimes your code will end up running when the readystate is interactive (just before DOMContentLoaded), or when it's complete (if their code runs too late for DOMContentLoaded, and the window.load case is hit instead), etc.

The end result is that with your code the way it was before, Tablesaw's code might think it's time to initialize right away, before you ran ajaxUpdate and had the table ready for it.

Given all of the code involved, I honestly can't tell you why the problem feels more common now than before; there are just too many moving parts. But I tried a mozregression, and the problem was easily triggered way back in Firefox 80, so this isn't something new on Firefox's end, from what I can tell. We'd need someone who used to have no problem, but does now, to try mozregression on your old site, and hope there really was a specific change in Firefox itself that caused the problem to get worse.

Comment 11

[adam]

Hmm, that's none too clever of jQuery is it! If we want to run something when the document is ready then it should run when it's ready, never before it's ready. In this case however the table to which Tablesaw is being applied is included in the page load - just without the prices in tbody. The ajax call just fetches an updated tbody (prices) based on selections. So something deeper is happening as the table is there when called.

I suspect it's the change in placement of the script at the end of the document now causing this. This is Magento's doing and we cannot change/control it. As Tablesaw hasn't been updated in a while I've simplified the init to just add an event listener, regardless of readystate. That does the job :)

Thanks for your time on this.

Comment 12

[Thomas Wisniewski [:twisniewski]]

Right, thanks for letting us know, it will be good to keep an eye on things to see if we need to worry about Magento.

Note that jQuery isn't really to blame here, because they can't ensure that their script will load during any specific phase of the page-load (plus they have hacks for older browser bugs too, so.. yeah. This is all just complicated stuff, especially once you have multiple libraries doing their own ready-handling).

I just noticed your attempt to patch around Tablesaw's init, thanks for the effort! With luck it will be enough (hopefully this is the only library you have where this kind of thing might be an issue :S)