Problem/Motivation
The validation for path aliases should be the same whether saving through the admin interface (admin/config/search/path) or node edit forms. As identified in #1847174: Path alias validation should test for relative path, no trailing slash requirements, however, each of those has its own completely separate validation handler and automated tests, and they all behave a little different:
| Validation handler | Automated tests | |
|---|---|---|
| Admin form | path_admin_form_validate() | PathAliasTest::testAdminAlias() |
| Node edit form | path_form_element_validate() | PathAliasTest::testNodeAlias() |
Proposed resolution
The validation logic should be consolidated--perhaps extracted into a nice, unit-testable utility function. The duplicativeness of the automated tests should be addressed, as well.
Remaining tasks
Fix, review, etc.
User interface changes
None.
API changes
tbd
Comments
Comment #4
dawehnerI think we have a bit of a big problem here. Let me update the issue summary what this leads to.
I set it to critical as you can easily destroy the complete Drupal UI.
Comment #5
catchThis looks like a partial duplicate of #121362: Require dedicated permission(s) to set aliases pointing to existing or reserved paths
Comment #6
catchDiscussed this with xjm, effulgentsia and cottser. We agreed the issue of overriding existing paths is a duplicate of #121362: Require dedicated permission(s) to set aliases pointing to existing or reserved paths so removing that from the summary.
Unifying the validation and test coverage seems like a good normal task to do, so downgrading to that.
Comment #12
danflanagan8I wonder what the status of this issue is...
path_form_element_validate()appears to have been removed in #2201051: Convert path.module form alters to a field widgetpath_admin_form_validate()appears to have been removed in #1987802: Convert path_admin_overview() to a new style controllerSo the IS at least is outdated.
Digging deeper, both the Admin form and the Node edit form use the PathWidget nowadays. The Node edit form was modified to use the widget in #2201051: Convert path.module form alters to a field widget. The admin form was modified to use the widget in #3007832: Convert custom path alias forms to entity forms.
It looks to me that both the Node edit and the Admin form are using the same validation these days, so I'm going to mark this as outdated.
As for the test coverage, I'm of the mind that there's no harm in testing both the Node form and the Admin form. There's more than validation being tested in the
PathAliasTestfunctional tests.If anyone wants to re-open, please do so with details. Thanks!