Skip to content

Commit

Permalink
Remove setNamespace() restriction and improve Html::id() instead
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonkelly committed Dec 14, 2023
1 parent cedd0ba commit aa18b0f
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 63 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG-WIP.md
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@
- `craft\gql\mutations\Entry::createSaveMutations()` now accepts a `$section` argument.
- `craft\helpers\Cp::fieldHtml()` now supports a `labelExtra` config value.
- `craft\helpers\Db::parseParam()`, `parseDateParam()`, `parseMoneyParam()`, and `parseNumericParam()` now return `null` instead of an empty string if no condition should be applied.
- `craft\helpers\Html::id()` and `Craft.formatInputId()` now retain colons and periods, and ensure the string begins with a letter.
- `craft\helpers\Html::normalizeTagAttributes()` now supports a `removeClass` key.
- `craft\helpers\Html::tag()` and `beginTag()` now ensure that the passed-in attributes are normalized.
- `craft\helpers\StringHelper::toString()` now supports backed enums.
Expand All @@ -331,7 +332,6 @@
- `craft\services\Users::unshunMessageForUser()` now has a `void` return type, and throws an `InvalidElementException` in case of failure.
- `craft\services\Users::unsuspendUser()` now has a `void` return type, and throws an `InvalidElementException` in case of failure.
- `craft\services\Users::verifyEmailForUser()` now has a `void` return type, and throws an `InvalidElementException` in case of failure.
- `craft\web\View::setNamespace()` now throws an `InvalidArgumentException` for namespaces that don’t confirm to HTML `id` attribute rules (possibly followed by sets of properly-formatted strings wrapped in square brackets). ([#13943](https://github.com/craftcms/cms/issues/13943))
- Deprecated the `_elements/element.twig` control panel template. `elementChip()` or `elementCard()` should be used instead.
- Deprecated the `cp.elements.element` control panel template hook.
- Deprecated `craft\events\DefineElementInnerHtmlEvent`.
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Added `craft\helpers\Cp::moneyFieldHtml()`.
- Added `craft\helpers\Cp::moneyInputHtml()`.
- `craft\helpers\Html::id()` and `Craft.formatInputId()` now retain colons and periods, and ensure the string begins with a letter.
- `craft\web\View::setNamespace()` is no longer strict about namespaces matching HTML `id` attribute rules. ([#13943](https://github.com/craftcms/cms/issues/13943)

## 5.0.0-alpha.1 - 2023-12-13

Expand Down
4 changes: 3 additions & 1 deletion src/helpers/Html.php
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,9 @@ public static function unwrapNoscript(string $content): array
*/
public static function id(string $id = ''): string
{
$id = trim(preg_replace('/[^\w]+/', '-', $id), '-');
// IDs must begin with a letter
$id = preg_replace('/^[^A-Za-z]+/', '', $id);
$id = rtrim(preg_replace('/[^A-Za-z0-9_:.]+/', '-', $id), '-');
return $id ?: StringHelper::randomString(10);
}

Expand Down
8 changes: 0 additions & 8 deletions src/web/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
use Twig\TemplateWrapper;
use yii\base\Arrayable;
use yii\base\Exception;
use yii\base\InvalidArgumentException;
use yii\base\Model;
use yii\base\NotSupportedException;
use yii\web\AssetBundle as YiiAssetBundle;
Expand Down Expand Up @@ -1439,13 +1438,6 @@ public function getNamespace(): ?string
*/
public function setNamespace(?string $namespace): void
{
if (
$namespace !== null &&
!preg_match('/^(__.+__|[A-Za-z][A-Za-z0-9\-_:.]*(\[[A-Za-z][A-Za-z0-9\-_:.]*])*)$/', $namespace)
) {
throw new InvalidArgumentException(sprintf('Invalid namespace ("%s"). Namespaces must begin with a letter, and may be followed by letters, numbers, hyphens, underscores, colons, and periods (possibly followed by sets of correctly-formatted strings wrapped in square brackets).', $namespace));
}

$this->_namespace = $namespace;
}

Expand Down
2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js.map

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion src/web/assets/cp/src/js/Craft.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,10 @@ $.extend(Craft, {
* @returns {string}
*/
formatInputId: function (inputName) {
return this.rtrim(inputName.replace(/[^\w\-]+/g, '-'), '-');
// IDs must begin with a letter
let id = inputName.replace(/^[^A-Za-z]+/, '');
id = this.rtrim(id.replace(/[^A-Za-z0-9_:.]+/g, '-'), '-');
return id || this.randomString(10);
},

/**
Expand Down
13 changes: 10 additions & 3 deletions tests/unit/helpers/HtmlHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,16 @@ public function testNormalizeTagAttributes(array $expected, array $attributes):

/**
* @dataProvider idDataProvider
* @param string $expected
* @param string|null $expected
* @param string $id
*/
public function testId(string $expected, string $id): void
public function testId(?string $expected, string $id): void
{
self::assertSame($expected, Html::id($id));
if ($expected) {
self::assertSame($expected, Html::id($id));
} else {
self::assertEquals(10, strlen(Html::id($id)));
}
}

/**
Expand Down Expand Up @@ -485,6 +489,9 @@ public static function idDataProvider(): array
['foo-bar', 'foo--bar'],
['foo-bar-baz', 'foo[bar][baz]'],
['foo-bar-baz', 'foo bar baz'],
['foo.bar', 'foo.bar'],
['foo-bar', 'foo bar'],
[null, '100'],
];
}

Expand Down
47 changes: 0 additions & 47 deletions tests/unit/web/ViewTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
use UnitTester;
use yii\base\Event;
use yii\base\Exception;
use yii\base\InvalidArgumentException;

/**
* Unit tests for the View class
Expand Down Expand Up @@ -308,26 +307,6 @@ public function testNamespaceInputId(string $expected, string $string, ?string $
self::assertSame($expected, $this->view->namespaceInputId($string, $namespace));
}

/**
* @dataProvider setNamespaceDataProvider
* @param string|null $namespace
* @param bool $isValid
*/
public function testSetNamespace(?string $namespace, bool $isValid): void
{
$oldNamespace = $this->view->getNamespace();

if (!$isValid) {
self::expectException(InvalidArgumentException::class);
}

$this->view->setNamespace($namespace);
self::assertEquals($namespace, $this->view->getNamespace());

$this->view->setNamespace($oldNamespace);
self::assertEquals($oldNamespace, $this->view->getNamespace());
}

/**
* @dataProvider getTemplateRootsDataProvider
* @param array $expected
Expand Down Expand Up @@ -623,32 +602,6 @@ public static function namespaceInputIdDataProvider(): array
];
}

/**
* @return array
*/
public static function setNamespaceDataProvider(): array
{
return [
[null, true],
['foo', true],
['foo[bar]', true],
['foo[bar][baz]', true],
['foo[bar0:baz.1-_]', true],
['', false],
['0', false],
['1', false],
['foo[]', false],
['foo[0]', false],
['foo[1]', false],
['foo[bar][]', false],
['foo[bar][0]', false],
['foo[bar][1]', false],
['foo[bar', false],
[' foo', false],
['__FOO__', true],
];
}

/**
* @return array
*/
Expand Down

0 comments on commit aa18b0f

Please sign in to comment.