Merge branch '5' into 6

.github/workflows/dispatch-ci.yml

@@ -1,16 +1,21 @@
 name: Dispatch CI
 
 on:
-  # At 2:20 PM UTC, only on Tuesday and Wednesday
+  # At 8:40 AM UTC, only on Wednesday and Thursday
   schedule:
-    - cron: '20 14 * * 2,3'
+    - cron: '40 8 * * 3,4'
+
+permissions: {}
 
 jobs:
   dispatch-ci:
     name: Dispatch CI
     # Only run cron on the silverstripe account
     if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      actions: write
     steps:
       - name: Dispatch CI
         uses: silverstripe/gha-dispatch-ci@v1

.github/workflows/keepalive.yml

@@ -1,17 +1,21 @@
 name: Keepalive
 
 on:
-  # At 1:05 PM UTC, on day 22 of the month
+  # At 5:25 AM UTC, on day 18 of the month
   schedule:
-    - cron: '5 13 22 * *'
+    - cron: '25 5 18 * *'
   workflow_dispatch:
 
+permissions: {}
+
 jobs:
   keepalive:
     name: Keepalive
     # Only run cron on the silverstripe account
     if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
     runs-on: ubuntu-latest
+    permissions:
+      actions: write
     steps:
       - name: Keepalive
         uses: silverstripe/gha-keepalive@v1

.github/workflows/merge-up.yml

@@ -1,17 +1,22 @@
 name: Merge-up
 
 on:
-  # At 2:20 PM UTC, only on Saturday
+  # At 8:40 AM UTC, only on Sunday
   schedule:
-    - cron: '20 14 * * 6'
+    - cron: '40 8 * * 0'
   workflow_dispatch:
 
+permissions: {}
+
 jobs:
   merge-up:
     name: Merge-up
     # Only run cron on the silverstripe account
     if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
     runs-on: ubuntu-latest
+    permissions:
+      contents: write
+      actions: write
     steps:
       - name: Merge-up
         uses: silverstripe/gha-merge-up@v1

sake

@@ -9,7 +9,8 @@ Executes a SilverStripe command"
 	exit 1
 fi
 
-if ! [ -x "$(command -v which)" ]; then
+command -v which >/dev/null 2>&1
+if [ $? -ne 0 ]; then
   echo "Error: sake requires the 'which' command to operate." >&2
   exit 1
 fi

src/Forms/FieldList.php

@@ -7,6 +7,7 @@
 use SilverStripe\Forms\Tab;
 use SilverStripe\Forms\TabSet;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\Dev\Deprecation;
 
 /**
  * A list designed to hold form field instances.
@@ -211,6 +212,10 @@ public function addFieldToTab(string $tabName, FormField $field, ?string $insert
      */
     public function addFieldsToTab(string $tabName, array $fields, ?string $insertBefore = null): static
     {
+        if (!is_array($fields)) {
+            Deprecation::notice('5.3.0', '$fields will need to be passed as an array in CMS 6', Deprecation::SCOPE_METHOD);
+        }
+
         $this->flushFieldsCache();
 
         // Find the tab
@@ -256,6 +261,10 @@ public function removeFieldFromTab(string $tabName, string $fieldName): static
      */
     public function removeFieldsFromTab(string $tabName, array $fields): static
     {
+        if (!is_array($fields)) {
+            Deprecation::notice('5.3.0', '$fields will need to be passed as an array in CMS 6', Deprecation::SCOPE_METHOD);
+        }
+
         $this->flushFieldsCache();
 
         // Find the tab

src/Forms/GridField/GridField.php

@@ -276,9 +276,18 @@ public function performReadonlyTransformation()
             }
         }
 
-        // If the edit button has been removed, replace it with a view button
+        // If the edit button has been removed, replace it with a view button if one is allowed
         if ($hadEditButton && !$copyConfig->getComponentByType(GridFieldViewButton::class)) {
-            $copyConfig->addComponent(GridFieldViewButton::create());
+            $viewButtonClass = null;
+            foreach ($allowedComponents as $componentClass) {
+                if (is_a($componentClass, GridFieldViewButton::class, true)) {
+                    $viewButtonClass = $componentClass;
+                    break;
+                }
+            }
+            if ($viewButtonClass) {
+                $copyConfig->addComponent($viewButtonClass::create());
+            }
         }
 
         $copy->extend('afterPerformReadonlyTransformation', $this);

src/Forms/HTMLEditor/HTMLEditorField.php

@@ -192,7 +192,19 @@ public function getSchemaStateDefaults()
      */
     public function ValueEntities()
     {
-        return htmlentities($this->Value() ?? '', ENT_COMPAT, 'UTF-8', false);
+        $entities = get_html_translation_table(HTML_ENTITIES);
+
+        foreach ($entities as $key => $value) {
+            $entities[$key] = "/" . $value . "/";
+        }
+
+        $value = preg_replace_callback($entities, function ($matches) {
+            // Don't apply double encoding to ampersand
+            $doubleEncoding = $matches[0] != '&';
+            return htmlentities($matches[0], ENT_COMPAT, 'UTF-8', $doubleEncoding);
+        }, $this->Value() ?? '');
+
+        return $value;
     }
 
     /**

src/View/SSTemplateParser.peg

@@ -1031,13 +1031,13 @@ class SSTemplateParser extends Parser implements TemplateParser
     function ClosedBlock_Handle_Loop(&$res)
     {
         if ($res['ArgumentCount'] > 1) {
-            throw new SSTemplateParseException('Either no or too many arguments in control block. Must be one ' .
-                'argument only.', $this);
+            throw new SSTemplateParseException('Too many arguments in control block. Must be one or no' .
+                'arguments only.', $this);
         }
 
         //loop without arguments loops on the current scope
         if ($res['ArgumentCount'] == 0) {
-            $on = '$scope->obj(\'Up\', null)->obj(\'Foo\', null)';
+            $on = '$scope->locally()->obj(\'Me\', null, true)';
         } else {    //loop in the normal way
             $arg = $res['Arguments'][0];
             if ($arg['ArgumentMode'] == 'string') {

src/View/SSTemplateParser.php

@@ -1886,6 +1886,8 @@ function PresenceCheck_Argument(&$res, $sub)
             $res['php'] .= '((bool)'.$sub['php'].')';
         } else {
             $php = ($sub['ArgumentMode'] == 'default' ? $sub['lookup_php'] : $sub['php']);
+            // TODO: kinda hacky - maybe we need a way to pass state down the parse chain so
+            // Lookup_LastLookupStep and Argument_BareWord can produce hasValue instead of XML_val
             $res['php'] .= str_replace('$$FINAL', 'hasValue', $php ?? '');
         }
     }
@@ -4257,13 +4259,13 @@ function ClosedBlock__finalise(&$res)
     function ClosedBlock_Handle_Loop(&$res)
     {
         if ($res['ArgumentCount'] > 1) {
-            throw new SSTemplateParseException('Either no or too many arguments in control block. Must be one ' .
-                'argument only.', $this);
+            throw new SSTemplateParseException('Too many arguments in control block. Must be one or no' .
+                'arguments only.', $this);
         }
 
         //loop without arguments loops on the current scope
         if ($res['ArgumentCount'] == 0) {
-            $on = '$scope->obj(\'Up\', null)->obj(\'Foo\', null)';
+            $on = '$scope->locally()->obj(\'Me\', null, true)';
         } else {    //loop in the normal way
             $arg = $res['Arguments'][0];
             if ($arg['ArgumentMode'] == 'string') {
@@ -5290,6 +5292,8 @@ function Text__finalise(&$res)
         $text = stripslashes($text ?? '');
         $text = addcslashes($text ?? '', '\'\\');
 
+        // TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this
+        // non-dynamically calculated
         $code = <<<'EOC'
 (\SilverStripe\View\SSViewer::getRewriteHashLinksDefault()
     ? \SilverStripe\Core\Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) )
@@ -5328,7 +5332,8 @@ public function compileString($string, $templateName = "", $includeDebuggingComm
 
             $this->includeDebuggingComments = $includeDebuggingComments;
 
-            // Ignore UTF8 BOM at beginning of string.
+            // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF
+            // (and other encodings) properly
             if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) {
                 $this->pos = 3;
             }

src/View/Shortcodes/EmbedShortcodeProvider.php

@@ -171,6 +171,10 @@ protected static function videoEmbed($arguments, $content)
             $arguments['style'] = 'width: ' . intval($arguments['width']) . 'px;';
         }
 
+        if (!empty($arguments['caption'])) {
+            $arguments['caption'] = htmlentities($arguments['caption'], ENT_QUOTES, 'UTF-8', false);
+        }
+
         // override iframe dimension attributes provided by webservice with ones specified in shortcode arguments
         foreach (['width', 'height'] as $attr) {
             if (!($value = $arguments[$attr] ?? false)) {

templates/SilverStripe/View/Shortcodes/EmbedShortcodeProvider_video.ss

@@ -3,6 +3,6 @@
 >
     {$Content}
     <% if $Arguments.caption %>
-        <p class="caption">{$Arguments.caption}</p>
+        <p class="caption">{$Arguments.caption.RAW}</p>
     <% end_if %>
 </div>

tests/php/Forms/GridField/GridFieldReadonlyTest.php

@@ -20,6 +20,8 @@
 use SilverStripe\Forms\Tests\GridField\GridFieldTest\Team;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\GridField\GridFieldViewButton;
+use SilverStripe\Forms\Tests\GridField\GridFieldReadonlyTest\GridFieldViewButtonReplacement;
 use SilverStripe\Versioned\VersionedGridFieldState\VersionedGridFieldState;
 
 class GridFieldReadonlyTest extends SapphireTest
@@ -31,11 +33,28 @@ class GridFieldReadonlyTest extends SapphireTest
         Cheerleader::class,
     ];
 
+    public function provideReadOnlyTransformation(): array
+    {
+        return [
+            [
+                'viewButtonClass' => null,
+            ],
+            [
+                'viewButtonClass' => GridFieldViewButton::class,
+            ],
+            [
+                'viewButtonClass' => GridFieldViewButtonReplacement::class,
+            ],
+        ];
+    }
+
     /**
      * The CMS can set the value of a GridField to be a hasMany relation, which needs a readonly state.
      * This test ensures GridField has a readonly transformation.
+     *
+     * @dataProvider provideReadOnlyTransformation
      */
-    public function testReadOnlyTransformation()
+    public function testReadOnlyTransformation(?string $viewButtonClass)
     {
         // Build a hasMany Relation via getComponents like ModelAdmin does.
         $components = Team::get_one(Team::class)
@@ -67,6 +86,18 @@ public function testReadOnlyTransformation()
             $gridConfig
         );
 
+        if ($viewButtonClass !== GridFieldViewButton::class) {
+            $allowedComponents = $gridField->getReadonlyComponents();
+            $viewButtonIndex = array_search(GridFieldViewButton::class, $allowedComponents);
+            if ($viewButtonIndex !== false) {
+                unset($allowedComponents[$viewButtonIndex]);
+            }
+            if ($viewButtonClass !== null) {
+                $allowedComponents[] = $viewButtonClass;
+            }
+            $gridField->setReadonlyComponents($allowedComponents);
+        }
+
         // Model Admin sets the value of the GridField directly to the relation, which doesn't have a forTemplate()
         // function, if we rely on FormField to render into a ReadonlyField we'll get an error as HasManyRelation
         // doesn't have a forTemplate() function.

tests/php/Forms/GridField/GridFieldReadonlyTest/GridFieldViewButtonReplacement.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace SilverStripe\Forms\Tests\GridField\GridFieldReadonlyTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\Forms\GridField\GridFieldViewButton;
+
+class GridFieldViewButtonReplacement extends GridFieldViewButton implements TestOnly
+{
+
+}