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

Fix #11780: Fix resource handling to allow raw JS code #11784

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

melloware
Copy link
Member

Fix #11780: Fix resource handling to allow raw JS code

@tandraschko what happens is for some reason MyFaces sends this and Mojarra doesn't.

<update id="javax.faces.Resource"><![CDATA[
            console.log('hi');
        ]]></update>

When you try and convert that to Jquery var $content = $(content); it throws an error because that is not valid Jquery content. So I had to add a catch and say add the raw content to the head if an error is thrown.

@melloware melloware added the 🐞 defect Bug...Something isn't working label Apr 19, 2024
@tandraschko
Copy link
Member

dont like that try/catch TBH
i would check for a src attr

@melloware
Copy link
Member Author

I am struggling with how though as content is just a string value? And when you do $(content) JQuery throws an error.

@melloware
Copy link
Member Author

melloware commented Apr 19, 2024

I am checking for sources and links here...

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

But the catch is necessary for something that is not even a HTML element like console.log().

@tandraschko
Copy link
Member

maybe we should pass the node then and access via XML api

@melloware
Copy link
Member Author

Let me poke around with that idea of using the node.

@melloware
Copy link
Member Author

OK this is ugly. the content is always a single DOM node withe CDATA.

When you try and DOM parse it it fails the same way JQuery fails.

        // Create a new DOMParser
        var parser = new DOMParser();

        var parseXML = "<![CDATA[\n            console.log('hi');\n        ]]>";
        console.log(parseXML);
        // Parse the CDATA content as XML
        var xmlDoc = parser.parseFromString(parseXML, "text/xml");

        // Access the nodes within the XML document
        var nodes = xmlDoc.documentElement.childNodes;

        // Log the nodes
        nodes.forEach(node => {
            console.log(node);
        });

Because CDATA is a valid node but the XML inside it is NOT valid XML "<![CDATA[\n console.log('hi');\n ]]>"

@tandraschko
Copy link
Member

Do you have a reproducer for the original problem? Maybe we should just revert and fix the root problem

@melloware
Copy link
Member Author

The original issue I can definitely watch happen in real time but having trouble isolating why. But this user from 6 years ago has the exact same issue: https://forum.primefaces.org/viewtopic.php?t=71210

And he couldn't figure out why but for some reason JSF thinks it needs to send the JS and CSS for a component again after its been loaded, unloaded, and reloaded all while on the same page.

@melloware
Copy link
Member Author

i will revert for now and re-open that ticket.

@melloware
Copy link
Member Author

Also I would like to understand why MyFaces duplicates the script in head and eval.

<?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>

@tandraschko
Copy link
Member

Both are propably bugs in the impls, we should create issues there

@melloware
Copy link
Member Author

ok let me create issues there.

@melloware
Copy link
Member Author

melloware commented Apr 19, 2024

Actually I started with OmniFaces since the <o:onloadScript> is what triggered this to begin with: omnifaces/omnifaces#804

@melloware melloware merged commit a988af5 into primefaces:master Apr 19, 2024
12 checks passed
@melloware melloware deleted the PR11780 branch April 19, 2024 20:41
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 this pull request may close these issues.

Cannot upgrade to 13.0.8. Some strange behaviour with omnifaces
2 participants