Repository: primefaces/primefaces (java)

Issue:
primefaces.MOVE_SCRIPTS_TO_BOTTOM: Import with additional parameters
**Description**
The function primefaces.MOVE_SCRIPTS_TO_BOTTOM, doesn't move addtional parameters of an script import to the bottom.
It works well when a <script> tag has only src / type attributes, but everything else will be deleted during the move process.

For example:

XHTML:
<script src="https://primefaces.org/primefaces.js"
type="text/javascript" charset="UTF-8"
data-domain-script="OneParameterValue" />

With primefaces.MOVE_SCRIPTS_TO_BOTTOM=off the output in the browser is:
<script src="https://primefaces.org/primefaces.js" 
type="text/javascript" charset="UTF-8" data-domain-script="OneParameterValue"></script>

With primefaces.MOVE_SCRIPTS_TO_BOTTOM=on:
<script type="text/javascript" src="https://primefaces.org/primefaces.js">

**Describe the solution you would like**
There are two possible solutions:
1. Skip imports with additional parameters
2. Change the move process, that addtional parameters are considered.


Pull request description:
Fix #7882: MoveScriptsToBottom include script attributes
@tandraschko not only did we already have a unit test ready to go but while testing the Showcase I can already see it working.

Out on PrimeFaces Showcase I see this URL...

```xml
<script type="text/javascript" src="https://www.googletagmanager.com/gtag/js?id=UA-93461466-1">
```

Running localhost with this change it has the proper async...

```xml
<script async="async" src="https://www.googletagmanager.com/gtag/js?id=UA-93461466-1" type="text/javascript">
```

```java
    @Test
    public void testPassthroughAttributes() throws IOException {
        writer.startElement("body", null);
        verify(wrappedWriter).startElement("body", null);

        writer.startElement("script", null);
        writer.writeAttribute("type", "text/javascript", null);
        writer.writeAttribute("src", "include", null);
        writer.writeAttribute("async", "true", null);
        writer.writeAttribute("defer", "true", null);
        writer.endElement("script");

        writer.endElement("body");

        verify(wrappedWriter).startElement("script", null);
        verify(wrappedWriter).writeAttribute("type", "text/javascript", null);
        verify(wrappedWriter).writeAttribute("src", "include", null);
        verify(wrappedWriter).writeAttribute("async", "true", null);
        verify(wrappedWriter).writeAttribute("defer", "true", null);
        verify(wrappedWriter).endElement("script");
    }
```

Patch:
diff --git a/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriter.java b/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriter.java
index 8f3cef277b..26a6aa22bc 100644
--- a/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriter.java
+++ b/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriter.java
@@ -23,31 +23,36 @@
  */
 package org.primefaces.application.resource;
 
-import org.primefaces.util.AgentUtils;
-import org.primefaces.util.LangUtils;
-
-import javax.faces.component.UIComponent;
-import javax.faces.context.FacesContext;
-import javax.faces.context.ResponseWriter;
-import javax.faces.context.ResponseWriterWrapper;
 import java.io.IOException;
 import java.io.Writer;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.UUID;
 
+import javax.faces.component.UIComponent;
+import javax.faces.context.FacesContext;
+import javax.faces.context.ResponseWriter;
+import javax.faces.context.ResponseWriterWrapper;
+
+import org.primefaces.util.AgentUtils;
+import org.primefaces.util.LangUtils;
+
 public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
 
     private static final String SCRIPT_TAG = "script";
     private static final String BODY_TAG = "body";
     private static final String HTML_TAG = "html";
+    private static final String SCRIPT_TYPE = "text/javascript";
+    private static final String TYPE_ATTRIBUTE = "type";
 
     private final ResponseWriter wrapped;
     private final MoveScriptsToBottomState state;
 
     private boolean inScript;
     private String scriptType;
-    private StringBuilder include;
+    private Map<String, String> includeAttributes;
     private StringBuilder inline;
     private boolean scriptsRendered;
 
@@ -62,7 +67,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
         scriptsRendered = false;
         writeFouc = false;
 
-        include = new StringBuilder(50);
+        includeAttributes = new LinkedHashMap<>(6);
         inline = new StringBuilder(75);
     }
 
@@ -144,7 +149,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
     @Override
     public void writeAttribute(String name, Object value, String property) throws IOException {
         if (inScript) {
-            updateScriptSrcOrType(name, (String) value);
+            updateAttributes(name, (String) value);
         }
         else {
             getWrapped().writeAttribute(name, value, property);
@@ -154,7 +159,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
     @Override
     public void writeURIAttribute(String name, Object value, String property) throws IOException {
         if (inScript) {
-            updateScriptSrcOrType(name, (String) value);
+            updateAttributes(name, (String) value);
         }
         else {
             getWrapped().writeURIAttribute(name, value, property);
@@ -165,7 +170,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
     public void startElement(String name, UIComponent component) throws IOException {
         if (SCRIPT_TAG.equalsIgnoreCase(name)) {
             inScript = true;
-            scriptType = "text/javascript";
+            scriptType = SCRIPT_TYPE;
         }
         else {
             writeFouc();
@@ -184,27 +189,33 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
             inScript = false;
 
             state.addInline(scriptType, inline);
-            state.addInclude(scriptType, include);
+            if (LangUtils.isNotBlank(includeAttributes.get("src"))) {
+                state.addInclude(scriptType, includeAttributes);
+            }
 
             scriptType = null;
-            include.setLength(0);
+            includeAttributes.clear();
             inline.setLength(0);
         }
         else if (BODY_TAG.equalsIgnoreCase(name) || (HTML_TAG.equalsIgnoreCase(name) && !scriptsRendered)) {
 
             // write script includes
-            for (Map.Entry<String, List<String>> entry : state.getIncludes().entrySet()) {
-                String type = entry.getKey();
-                List<String> includes = entry.getValue();
+            for (Entry<String, List<Map<String, String>>> entry : state.getIncludes().entrySet()) {
+                String scriptType = entry.getKey();
+                List<Map<String, String>> includes = entry.getValue();
 
                 for (int i = 0; i < includes.size(); i++) {
-                    String src = includes.get(i);
-                    if (src != null && !src.isEmpty()) {
-                        getWrapped().startElement(SCRIPT_TAG, null);
-                        getWrapped().writeAttribute("type", type, null);
-                        getWrapped().writeAttribute("src", src, null);
-                        getWrapped().endElement(SCRIPT_TAG);
+                    Map<String, String> attributes = includes.get(i);
+                    attributes.put(TYPE_ATTRIBUTE, scriptType);
+                    getWrapped().startElement(SCRIPT_TAG, null);
+                    for (Entry<String, String> attribute : attributes.entrySet()) {
+                        String attributeName = attribute.getKey();
+                        String attributeValue = attribute.getValue();
+                        if (LangUtils.isNotBlank(attributeValue)) {
+                            getWrapped().writeAttribute(attributeName, attributeValue, null);
+                        }
                     }
+                    getWrapped().endElement(SCRIPT_TAG);
                 }
             }
 
@@ -219,7 +230,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
                 if (LangUtils.isNotBlank(merged)) {
                     getWrapped().startElement(SCRIPT_TAG, null);
                     getWrapped().writeAttribute("id", id, null);
-                    getWrapped().writeAttribute("type", type, null);
+                    getWrapped().writeAttribute(TYPE_ATTRIBUTE, type, null);
                     getWrapped().write(merged);
                     getWrapped().endElement(SCRIPT_TAG);
                 }
@@ -256,7 +267,7 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
         String minimized = script.toString();
 
         if (LangUtils.isNotBlank(minimized)) {
-            if ("text/javascript".equalsIgnoreCase(type)) {
+            if (SCRIPT_TYPE.equalsIgnoreCase(type)) {
                 minimized = minimized.replace(";;", ";");
 
                 if (minimized.contains("PrimeFaces")) {
@@ -283,13 +294,10 @@ public class MoveScriptsToBottomResponseWriter extends ResponseWriterWrapper {
         return getWrapped().cloneWithWriter(writer);
     }
 
-    protected void updateScriptSrcOrType(String name, String value) {
-        if ("src".equalsIgnoreCase(name)) {
-            if (LangUtils.isNotBlank(value)) {
-                include.append(value);
-            }
-        }
-        else if ("type".equalsIgnoreCase(name)) {
+    protected void updateAttributes(String name, String value) {
+        includeAttributes.put(name, value);
+
+        if (TYPE_ATTRIBUTE.equalsIgnoreCase(name)) {
             if (LangUtils.isNotBlank(value)) {
                 scriptType = value;
             }
diff --git a/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomState.java b/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomState.java
index 7b23d38100..19f465e323 100644
--- a/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomState.java
+++ b/primefaces/src/main/java/org/primefaces/application/resource/MoveScriptsToBottomState.java
@@ -24,16 +24,13 @@
 package org.primefaces.application.resource;
 
 import java.io.Serializable;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 public class MoveScriptsToBottomState implements Serializable {
 
     private static final long serialVersionUID = 1L;
 
-    private Map<String, List<String>> includes;
+    private Map<String, List<Map<String, String>>> includes;
     private Map<String, List<String>> inlines;
     private int savedInlineTags;
 
@@ -43,10 +40,10 @@ public class MoveScriptsToBottomState implements Serializable {
         savedInlineTags = -1;
     }
 
-    public void addInclude(String type, StringBuilder src) {
-        if (src.length() > 0) {
-            List<String> includeList = includes.computeIfAbsent(type, k -> new ArrayList<>(20));
-            includeList.add(src.toString());
+    public void addInclude(String type, Map<String, String> includeAttributes) {
+        if (includeAttributes.size() > 0) {
+            List<Map<String, String>> includeList = includes.computeIfAbsent(type, k -> new ArrayList<>(20));
+            includeList.add(new LinkedHashMap<>(includeAttributes));
         }
     }
 
@@ -59,7 +56,7 @@ public class MoveScriptsToBottomState implements Serializable {
         }
     }
 
-    public Map<String, List<String>> getIncludes() {
+    public Map<String, List<Map<String, String>>> getIncludes() {
         return includes;
     }
 


Test patch:
diff --git a/primefaces/src/test/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriterTest.java b/primefaces/src/test/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriterTest.java
index aef75d138e..3003308667 100644
--- a/primefaces/src/test/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriterTest.java
+++ b/primefaces/src/test/java/org/primefaces/application/resource/MoveScriptsToBottomResponseWriterTest.java
@@ -24,25 +24,22 @@
 package org.primefaces.application.resource;
 
 
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.contains;
+import static org.mockito.ArgumentMatchers.matches;
+import static org.mockito.Mockito.*;
+
+import java.io.IOException;
+
+import javax.faces.context.ResponseWriter;
+
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
 import org.mockito.InOrder;
 import org.mockito.Mockito;
 
-import javax.faces.context.ResponseWriter;
-import java.io.IOException;
-import org.mockito.ArgumentMatchers;
-
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.contains;
-import static org.mockito.ArgumentMatchers.matches;
-import static org.mockito.Mockito.inOrder;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-
 public class MoveScriptsToBottomResponseWriterTest {
 
     private MoveScriptsToBottomResponseWriter writer;
@@ -168,18 +165,18 @@ public class MoveScriptsToBottomResponseWriterTest {
 
         writer.startElement("script", null);
         writer.writeAttribute("type", "text/javascript", null);
+        writer.writeAttribute("src", "include", null);
         writer.writeAttribute("async", "true", null);
         writer.writeAttribute("defer", "true", null);
-        writer.writeText("someJS", null);
         writer.endElement("script");
 
         writer.endElement("body");
 
         verify(wrappedWriter).startElement("script", null);
         verify(wrappedWriter).writeAttribute("type", "text/javascript", null);
-        // FIXME we would expect the attributes to be passed through
-        // verify(wrappedWriter).writeAttribute("async", "true", null);
-        // verify(wrappedWriter).writeAttribute("defer", "true", null);
+        verify(wrappedWriter).writeAttribute("src", "include", null);
+        verify(wrappedWriter).writeAttribute("async", "true", null);
+        verify(wrappedWriter).writeAttribute("defer", "true", null);
         verify(wrappedWriter).endElement("script");
     }
 
@@ -235,6 +232,7 @@ public class MoveScriptsToBottomResponseWriterTest {
 
         writer.startElement("script", null);
         writer.writeAttribute("src", "include", null);
+        writer.writeAttribute("charset", "UTF-8", null);
         writer.endElement("script");
 
         writer.startElement("script", null);
@@ -247,6 +245,7 @@ public class MoveScriptsToBottomResponseWriterTest {
         Assertions.assertEquals(1, state.getInlines().get("text/javascript").size());
         verify(wrappedWriter, times(2)).startElement("script", null);
         verify(wrappedWriter).writeAttribute("src", "include", null);
+        verify(wrappedWriter).writeAttribute("charset", "UTF-8", null);
         verify(wrappedWriter).write(contains("inline"));
     }