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

Cannot upgrade to 13.0.8. Some strange behaviour with omnifaces #11780

Closed
cocorossello opened this issue Apr 19, 2024 · 9 comments · Fixed by #11784
Closed

Cannot upgrade to 13.0.8. Some strange behaviour with omnifaces #11780

cocorossello opened this issue Apr 19, 2024 · 9 comments · Fixed by #11784
Assignees
Labels
🐞 defect Bug...Something isn't working
Milestone

Comments

@cocorossello
Copy link
Contributor

cocorossello commented Apr 19, 2024

Describe the bug

This example works in PF 13.0.7 but does not work in 13.0.8

<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:h="http://xmlns.jcp.org/jsf/html"
      xmlns:p="http://primefaces.org/ui"
      xmlns:o="http://omnifaces.org/ui"
      xmlns:c="http://xmlns.jcp.org/jsp/jstl/core"
      xmlns:f="http://xmlns.jcp.org/jsf/core">
<f:view>
    <h:head></h:head>
    <h:body>
        <c:if test="true">

        </c:if>
        <o:onloadScript>
            console.log('hi');
        </o:onloadScript>
        <h:form>
            <h1>
                This example works in PF 13.0.7 but does not work in PF 13.0.8
            </h1>
            <p:autoComplete id="acSimple" value="#{autocompleteViewBean.value}"
                            completeMethod="#{autocompleteViewBean.complete}" scrollHeight="250"/>
        </h:form>
    </h:body>
</f:view>
</html>

In order for this to fail it has to have both a <c:if> and a <o:onloadScript>, so I'm not sure where is the bug, but it was working in previous versions.

I provided a minimal reproducer in https://github.com/cocorossello/primefaces-test

To run it:
mvn clean jetty:run -Pmyfaces23next

Then open http://localhost:8080/primefaces-test/ and check that the autocomplete doesn't work (the call is performed and I can see the results in the xhr response but the update is lost somehow).

Change PF version to 13.0.7 and it works again

Reproducer

No response

Expected behavior

No response

PrimeFaces edition

None

PrimeFaces version

13.0.8

Theme

No response

JSF implementation

MyFaces

JSF version

2.3-next-M7

Java version

17

Browser(s)

No response

@cocorossello cocorossello added ‼️ needs-triage Issue needs triaging 🐞 defect Bug...Something isn't working labels Apr 19, 2024
@DazzlingBob
Copy link

This issue occurs only with myfaces23 and myfaces23next. With mojarra23 it works as expected.
So it is implementation specific.

@melloware

This comment was marked as outdated.

@melloware
Copy link
Member

OK here is why. MyFaces puts it in both <eval> section and javax.faces.Resource section and Mojarra only puts it in the <eval> section.

Since I added this fix #11714 it must be what is causing the issue.

MyFaces

<?xml version="1.0" encoding="UTF-8"?>
<partial-response id="j_id__v_0">
	<changes>
		<update id="javax.faces.Resource"><![CDATA[
            console.log('hi');
        ]]></update>
		<update id="j_id_9:acSimple"><![CDATA[<ul class="ui-autocomplete-items ui-autocomplete-list ui-widget-content ui-widget ui-corner-all ui-helper-reset" role="listbox"><li id="j_id_9:acSimple_item_0" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 1" data-item-value="Result 1" data-item-class="">Result 1</li><li id="j_id_9:acSimple_item_1" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 2" data-item-value="Result 2" data-item-class="">Result 2</li><li id="j_id_9:acSimple_item_2" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 3" data-item-value="Result 3" data-item-class="">Result 3</li></ul>]]></update>
		<update id="j_id__v_0:javax.faces.ViewState:1"><![CDATA[YTFlMDk1NzRjYzBmMTlmYjAwMDAwMDAx]]></update>
		<eval><![CDATA[console.log('hi');]]></eval>
	</changes>
</partial-response>

Mojarra

<?xml version='1.0' encoding='utf-8'?>
<partial-response>
	<changes>
		<update id="j_idt6:acSimple"><![CDATA[<ul class="ui-autocomplete-items ui-autocomplete-list ui-widget-content ui-widget ui-corner-all ui-helper-reset" role="listbox"><li id="j_idt6:acSimple_item_0" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 1" data-item-value="Result 1">Result 1</li><li id="j_idt6:acSimple_item_1" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 2" data-item-value="Result 2">Result 2</li><li id="j_idt6:acSimple_item_2" class="ui-autocomplete-item ui-autocomplete-list-item ui-corner-all" data-item-label="Result 3" data-item-value="Result 3">Result 3</li></ul>]]></update>
		<update id="j_id1:javax.faces.ViewState:0"><![CDATA[-4627649845244533925:-1742591768271823828]]></update>
		<eval><![CDATA[console.log('hi');]]></eval>
	</changes>
</partial-response>

melloware added a commit to melloware/primefaces that referenced this issue Apr 19, 2024
@melloware melloware self-assigned this Apr 19, 2024
@melloware melloware added this to the 13.0.9 milestone Apr 19, 2024
@melloware melloware removed the ‼️ needs-triage Issue needs triaging label Apr 19, 2024
@melloware
Copy link
Member

PR submitted but if you need a MonkeyPatch right now:

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);
    }
  }
};

@cocorossello
Copy link
Contributor Author

Awesome, thanks. I can confirm that the monkeypatch works as expected

@melloware
Copy link
Member

I reverted the original change for now and have a question out to OmniFaces.

@mkomko
Copy link
Contributor

mkomko commented Apr 23, 2024

We ran into the same thing and had weird behavior at different places of our application. It took me a while to find the problematic code and then I luckily found this issue, otherwise it would have taken me even longer to reproduce it.

@melloware
Copy link
Member

yep i think we need to get to the bottom of why MyFaces and Mojarra do something different here and what the spec says etc. I have a note out to BalusC but he is currently on vacation so I reverted this for now.

@melloware
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 defect Bug...Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants