primefaces / primefaces

Ultimate Component Suite for JavaServer Faces
http://www.primefaces.org
MIT License
1.81k stars 763 forks source link

Core: AJAX update of resource doesn't check for duplicate CSS/JS #11714

Closed melloware closed 3 months ago

melloware commented 7 months ago

Describe the bug

@tandraschko this was originally reported on a forum post in 2022: https://forum.primefaces.org/viewtopic.php?t=71210

We never got to the bottom of what was happening then...but my current client has this same issue. They do a LOT of dynamic removing and adding components to a single page app using PrimeFaces.current().ajax().update() as well as dynamically constructing components in Java. its a HUGE project.

Here you can see duplicates in the head like InputNumber and FileUpload. If I close one of their panels and open it again it will add 2 more copies.

I shortened it for brevity but in javax.faces.Resource update the scripts are there again.

<?xml version='1.0' encoding='UTF-8'?>
<partial-response>
    <changes>
        <update id="javax.faces.Resource"><![CDATA[
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/fileupload/fileupload.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7" />
<script type="text/javascript" src="/emro/javax.faces.resource/fileupload/fileupload.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputnumber/inputnumber.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/inputnumber/inputnumber.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7" />
<script type="text/javascript" src="/emro/javax.faces.resource/inputmask/inputmask.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>]]></update>
        <update id="mainForm:mainPnl"><![CDATA[<div id="mainForm:mainPnl" </div>]]></update>
        <update id="j_id1:javax.faces.ViewState:0"><![CDATA[-1272965173743229381:1530783905304280517]]></update>
    </changes>
</partial-response>

Our core.ajax for those just automatically appends to the head without checking for existing CSS or JS already loaded.

else if (id === PrimeFaces.ajax.RESOURCE) {
    $('head').append(content);
}

And thus over time our head is filling with more and more scripts and is part of their overall browser slowdown after using the app for an hour...

<head id="j_idt2">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="apple-mobile-web-app-capable" content="yes">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0">
<link href="/emro/resources/css/fonts.css" rel="stylesheet" type="text/css">
<link href="/emro/resources/css/icons.css" rel="stylesheet" type="text/css">
<link rel="apple-touch-icon" href="/emro/resources/bmp/eMRO_WebIcon.png">
<link rel="shortcut icon" type="image/x-icon" href="/emro/resources/bmp/eMRO_Web.ico">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/blockui/blockui.css.trax?ln=primefaces-extensions&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/bootstrap.css.trax?ln=css">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/components.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/css/layout-blue.css.trax?ln=california-layout">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/css/nanoscroller.css.trax?ln=california-layout">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/fa/font-awesome.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/fileupload/fileupload.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/fileupload/fileupload.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/inputnumber/inputnumber.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/inputnumber/inputnumber.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/inputnumber/inputnumber.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/local.css.trax?ln=css">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/primeicons/primeicons.css.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7">
<link type="text/css" rel="stylesheet" href="/emro/javax.faces.resource/theme.css.trax?ln=primefaces-california-blue&amp;v=10.0.20&amp;e=10.0.7">

<script type="text/javascript" src="/emro/javax.faces.resource/blockui/blockui.js.trax?ln=primefaces-extensions&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/chartjs/chartjs.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/components.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/core.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/filedownload/filedownload.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/fileupload/fileupload.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/fileupload/fileupload.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputmask/inputmask.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputmask/inputmask.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputmask/inputmask.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputnumber/inputnumber.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputnumber/inputnumber.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/inputnumber/inputnumber.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/jquery/jquery-plugins.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/jquery/jquery.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/js/layout.js.trax?ln=california-layout"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/js/nanoscroller.js.trax?ln=california-layout"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/moment/moment.js.trax?ln=primefaces&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript" src="/emro/javax.faces.resource/primefaces-extensions.js.trax?ln=primefaces-extensions&amp;v=10.0.20&amp;e=10.0.7"></script>
<script type="text/javascript">if(window.PrimeFaces){PrimeFaces.settings.locale='en_US';PrimeFaces.settings.viewId='/Home.xhtml';PrimeFaces.settings.contextPath='/emro';PrimeFaces.settings.cookiesSecure=true;}</script>
</head>

Reproducer

I have no idea quite how to reproduce it but I am seeing it in their code base same as that user from 2022. They are not using a Custom HeadRenderer or anything.

Expected behavior

No duplicate CSS or JS even if the head is sent duplicate scripts.

PrimeFaces edition

Community

PrimeFaces version

13.0.0

Theme

California

JSF implementation

Mojarra

JSF version

2.3

Java version

11

Browser(s)

No response

melloware commented 6 months ago

I am still investigating this but I think it might be this Mojarra bug long since fixed that caused it to keep adding resources to the page that have already been added

https://github.com/javaee/mojarra/issues/4249

kaisernova commented 3 months ago

@melloware Hi, Issue is resolved in PF 13.0.8 BUT it appears again in PF 13.0.10 again!!!!! Maybe a bad merge??????

Tested today.

melloware commented 3 months ago

@kaisernova the fix was reverted and was not in 13.0.10 because in 13.0.8 it introduced these bugs: https://github.com/primefaces/primefaces/issues/11780

So we need to get to the bottom of this core issue before we attempt another fix.

So are you saying you see the double resources? Can you give me details about your environment and how I can reproduce it?

kaisernova commented 3 months ago

@melloware Let me explain our environment (Java8, JEE7, Mojarra 2.3.x):

  1. We have a page with a lot of elements (like selects, text inputs and dataTable).
  2. It happens when an selectOneMenu calls an p:ajax change event, and it updates (using the "update" attrib) some other elements in the page. Important: The ajax call is partial only.
  3. when the partial response returns, it contains the "javax.faces.resource" [update] with ALL the resources for the [head] element AGAIN, and this content is appended again to the existing [head] elements. It happens always in every request from the change event from the selectOneMenu.
  4. It not happens if I change the "update" attrib from the p:ajax for update all the page (BTW this is not what we need, its NOT optimal, because there are other several elements and a considerable big datatable)
  5. It works ok in Primefaces 8, but because other bug in datatables we updated it to PF10, but this new bug appears, then we tried PF11... PF12... and then I check this post searching in the web for help.
  6. It happens in all pages with similar behavior. I see this bug because it brokes some CSS styles in the page and looked some heavy responses in the Chrome DevTools. I asume there is a lot of other apps with this problem (It is not visual at all at first sight).

Thanks in advance for your help

tandraschko commented 3 months ago

sounds still like a broken view or a mojarra bug we would need a reproducer here and check the root problem, everything else is just hack on PF side

melloware commented 3 months ago

@kaisernova i totally feel your pain this is hard to replicate. I have one client on Mojarra 2.3.14 and its happening to them and they have a very complex app so i have not been able to nail down exactly what in Mojarra causes this.

2 things.

  1. Have you tried upgrading to Mojarra 2.3.21 which is the latest and last Mojarra 2.3X release?

  2. If this doesn't fix it here is a MonkeyPatch my client has been using for 3 months in Production and has not seen an issue since.

PrimeFaces.ajax.Utils.updateElement = function (id, content, xhr) {
  if (id.indexOf(PrimeFaces.VIEW_STATE) !== -1) {
    PrimeFaces.ajax.Utils.updateFormStateInput(PrimeFaces.VIEW_STATE, content, xhr);
  } else if (id.indexOf(PrimeFaces.CLIENT_WINDOW) !== -1) {
    PrimeFaces.ajax.Utils.updateFormStateInput(PrimeFaces.CLIENT_WINDOW, content, xhr);
  }
  // used by @all
  else if (id === PrimeFaces.VIEW_ROOT) {
    // backup our utils, we reset it soon
    var ajaxUtils = PrimeFaces.ajax.Utils;

    // reset PrimeFaces JS state because the view is completely replaced with a new one
    window.PrimeFaces.resetState();

    ajaxUtils.updateHead(content);
    ajaxUtils.updateBody(content);
  } else if (id === PrimeFaces.ajax.VIEW_HEAD) {
    PrimeFaces.ajax.Utils.updateHead(content);
  } else if (id === PrimeFaces.ajax.VIEW_BODY) {
    PrimeFaces.ajax.Utils.updateBody(content);
  } else if (id === PrimeFaces.ajax.RESOURCE) {
    var $head = $("head");
    try {
      var $content = $(content);
      var filteredContent = $content.length > 0 ? $content.filter("link[href], script[src]") : $();

      if (filteredContent.length === 0) {
        PrimeFaces.debug("Adding content to the head because it lacks any JavaScript or CSS links...");
        $head.append(content);
      } else {
        // #11714 Iterate through each script and stylesheet tag in the content
        // checking if resource is already attached to the head and adding it if not
        filteredContent.each(function () {
          var $resource = $(this);
          var src = $resource.attr("href") || $resource.attr("src");
          var type = this.tagName.toLowerCase();
          var $resources = $head.find(type + '[src="' + src + '"], ' + type + '[href="' + src + '"]');

          // Check if script or stylesheet already exists and add it to head if it does not
          if ($resources.length === 0) {
            PrimeFaces.debug("Appending " + type + " to head: " + src);
            $head.append($resource);
          }
        });
      }
    } catch (error) {
      PrimeFaces.debug("Adding content to the head since it consists solely of raw JavaScript code...");
      $head.append(content);
    }
  } else if (id === $("head")[0].id) {
    PrimeFaces.ajax.Utils.updateHead(content);
  } else {
    var target = $(PrimeFaces.escapeClientId(id));
    if (target.length === 0) {
      PrimeFaces.warn("DOM element with id '" + id + "' cant be found; skip update...");
    } else {
      var removed = target.replaceWith(content);
      PrimeFaces.utils.cleanseDomElement(removed);
    }
  }
};
kaisernova commented 3 months ago

@melloware @tandraschko Hi again, I have some good news for you (maybe): Checking https://javaee.github.io/javaserverfaces/whats-new-in-jsf23.html and the ajax and partial docs, I found that the problem was that the Partial State Saving was not working in the update (I dont know why).

Then, I put the following context param with the value as "true" in our web.xml:

`

javax.faces.PARTIAL_STATE_SAVING
    <param-value>true</param-value>

`

And the app works again like with the PF8 version. I checked the reponses and the "javax.faces.Resource" block don't appears again and the style in the page dont go crazy again.

Thanks for your attention, I hope this can help you.

melloware commented 3 months ago

Interesting from that doc you posted

The "jsf.js" script has to append every element from the update section with this new javax.faces.Resource identifier to the head of the HTML document, if that element is not already there Note that via the condition "if that element is not already there" in the above algorithm the "jsf.js" script now takes upon it a part of deduplicating resource references, a task that was previously done solely server side.

That is the part we are not seeing/doing in our PF JavaScript. I need to look at what JSF.js does

melloware commented 3 months ago

@tandraschko i think this is part of the Faces Spec and PrimeFaces is not adhering to it.

Mojarra and MyFaces both check and filters out duplicates that have already been loaded:

Mojarra: https://github.com/eclipse-ee4j/mojarra/blob/adf0de352ad764c1dc59264072654bc5334a46de/impl/src/main/resources/META-INF/resources/javax.faces/jsf-uncompressed.js#L655-L657

Myfaces: https://github.com/apache/myfaces/blob/2.3.x/api/src/main/javascript/META-INF/resources/myfaces/_impl/_util/_Dom.js#L1291

tandraschko commented 3 months ago

Didnt we apply and then revert? What was the reason?

melloware commented 3 months ago

We did because of a MyFaces bug which I document is the reason the catch is needed:

https://issues.apache.org/jira/browse/MYFACES-4378

} catch (error) {
      // MYFACES-4378 is incorrectly sending executable code here in the Resource section
      PrimeFaces.debug("Appending content to the head as it contains only raw JavaScript code...");
       $head.append(content);
}

This solves the issue you disagreed with using a catch but its the safest way to work around the MyFaces bug

kaisernova commented 3 months ago

@melloware Also, note that the response from server is overloaded with the "javax.faces.Resource" with data that I don't need again in the page (Heavy response vs a light one with only the required element changes).

Also, in order to save memory in the server, we will use the "javax.faces.FULL_STATE_SAVING_VIEW_IDS" for the pages with the issue instead the "PARTIAL_STATE_SAVING" param (https://balusc.omnifaces.org/2011/09/communication-in-jsf-20.html#ViewScopedFailsInTagHandlers).

melloware commented 3 months ago

@kaisernova yes according to the spec "javax.faces.Resource should only contain CSS or JS script links. No other code should be in that block.

Then its the job of JSF or PF to make sure it doesn't try to load any of that CSS or JS that is already loaded on the page and discard it.