Add #type => 'path' that accepts path but optionally stores URL object or route name and parameters

This is really cool!

1. +++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
   @@ -0,0 +1,104 @@
   +  public function getInfo() {
   +    $class = get_class($this);
   +    return array(
   +      '#input' => TRUE,
   +      '#size' => 60,
   +      '#maxlength' => 128,
   +      '#autocomplete_route_name' => FALSE,
   +      '#process' => array(
   +        array($class, 'processAutocomplete'),
   +        array($class, 'processAjaxForm'),
   +        array($class, 'processPattern'),
   +        array($class, 'processGroup'),
   +      ),
   +      '#pre_render' => array(
   +        array($class, 'preRenderTextfield'),
   +        array($class, 'preRenderGroup'),
   +      ),
   +      '#element_validate' => array(
   +        array($class, 'validateMatchedPath'),
   +      ),
   +      '#theme' => 'input__textfield',
   +      '#theme_wrappers' => array('form_element'),
   +    );
   

   This could do parent::getInfo() and just add the #element_validate

2. +++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
   @@ -0,0 +1,104 @@
   +      $uri = $GLOBALS['base_url'] . '/' . $element['#value'];
   +      $request = Request::create($uri);
   ...
   +      $route_provider = \Drupal::service('router.route_provider');
   +      $collection = $route_provider->getRouteCollectionForRequest($request);
   ...
   +        preg_match($collection->get($matched_route_name)->compile()->getRegex(), '/' . $element['#value'], $parameters);
   

   Wow, this is a heavy method, glad its on save, not runtime.

   I think this method needs some inline docs.

3. +++ b/core/modules/system/src/Tests/Form/FormTest.php
   @@ -292,6 +292,33 @@ function testCheckboxProcessing() {
   +  function testMatchedPathProcessing() {
   
   +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestMatchedPathForm.php
   @@ -0,0 +1,49 @@
   +class FormTestMatchedPathForm extends FormBase {
   

   I'd like to avoid adding more to FormTest. I like what we did in FormDefaultHandlersTest, where the test class implements FormInterface, and everything is self-contained

+++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
@@ -0,0 +1,92 @@
+      $uri = $GLOBALS['base_url'] . '/' . $element['#value'];

Can we get the base_url from the current Request object instead of directly from $GLOBALS?

+++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
@@ -0,0 +1,93 @@
+      $request = \Drupal::request();
+      $uri = $request->getSchemeAndHttpHost() . $request->getBasePath() . '/' . $element['#value'];
+      $mock_request = Request::create($uri);
+
+      /** @var \Drupal\Core\Routing\RouteProviderInterface $route_provider */
+      $route_provider = \Drupal::service('router.route_provider');
+      // Use the route provider to get routes matching the given mock request.
+      $collection = $route_provider->getRouteCollectionForRequest($mock_request);
+      if ($collection->count() == 0) {
+        $form_state->setError($element, t('There is no such path %path.', array(
+          '%path' => $uri,
+        )));
+      }
+      else {
+        // Extract the first matching route name and parameters from the
+        // collection.
+        $routes = $collection->all();
+        $route_names = array_keys($routes);
+        $matched_route_name = reset($route_names);
+        $values = array(
+          'route_name' => $matched_route_name,
+          'route_parameters' => array(),
+        );
+        $parameters = array();
+        // Compile the route and match the parameters.
+        preg_match($collection->get($matched_route_name)->compile()->getRegex(), '/' . $element['#value'], $parameters);
+        foreach ($parameters as $id => $value) {
+          if (preg_match('/^\d+$/', $id)) {
+            // Discard parameters with numeric keys.
+            unset($parameters[$id]);
+          }
+        }
+        // Set the relevant parameters if any.
+        $values['route_parameters'] = $parameters;
+        $form_state->setValueForElement($element, $values);
+      }

You should rather do something like this: \Drupal\Core\Url::createFromPath($element['#value']), with a try catch around it. then you can pull the route name + parameters from there.

1. +++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
   @@ -0,0 +1,70 @@
   +   * Form element validation handler for matched_path elements.
   +   *
   +   * Note that #maxlength is validated by _form_validate() already.
   +   *
   +   * This checks that the submitted value matches an active route.
   +   */
   +  public static function validateMatchedPath(&$element, FormStateInterface $form_state, &$complete_form) {
   

   Can we document the new methods as well?

2. +++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
   @@ -0,0 +1,70 @@
   +        $values = array(
   +          'route_name' => $url->getRouteName(),
   +          'route_parameters' => $url->getRouteParameters(),
   +        );
   +        $form_state->setValueForElement($element, $values);
   

   Did you considered to return an url object directly? We use it in a hell lot of places already. The concept would be kinda similar to an entity form which sets the entity itself onto the form state. Oppinions?

3. +++ b/core/lib/Drupal/Core/Render/Element/MatchedPath.php
   @@ -0,0 +1,70 @@
   +        $form_state->setError($element, t('There is no such path %path.', array(
   ...
   +        $form_state->setError($element, t('You cannot use an external URL, please enter a relative path.'));
   

   I wonder whether we could just use the StringTranslationTrait and use $this->t()

4. +++ b/core/modules/system/src/Tests/Form/FormTest.php
   @@ -292,6 +292,33 @@ function testCheckboxProcessing() {
   +  function testMatchedPathProcessing() {
   

   let's mark it as public

5. +++ b/core/modules/system/src/Tests/Form/FormTest.php
   @@ -292,6 +292,33 @@ function testCheckboxProcessing() {
   +    $values = Json::decode($this->drupalPostForm(NULL, array('required_matched_path' => 'user/1'), t('Submit')));
   

   Wow, this is a crazy trick!

It's a static method so we have no DI or string translation trait :(

Well, what about adding a follow up to provide a static version of the trait, so static::t() and static::formatPlural() would be supported? This could be for example also used for baseFieldDefinitions?

Adding two closely related issues.

Somehow I was under the impression that this form element #type would have existed in the past (~D5), but wasn't able to find it. In any case, +1.

This #type somewhat belongs to Link module + Menu* modules, as well as Path module. Due to that, it's probably good to define it globally in System module.

However, that also means that there are various possible incarnations of a "path" UI element/widget. We should investigate whether more specialized element use-cases can be derived from the base/generic element type, and ensure that the architecture and DX makes sense.

In addition, the HTML5 standard input form element #type 'url' co-exists next to this. Add path, link, menu_link, etc to that, and things get confusing very fast.

I'd start by renaming this new #type from 'matched_path' to just 'path': A form input element that accepts an internal system path.

Additionally matching an existing path against available routes is an act of user input (#element_validate) validation, which e.g. could be controlled by a #validate_route => TRUE/FALSE property on the element.

Additionally replacing a user-entered 'path' with the name of the matched route is an act of user input/form state #value post-processing, which e.g. could be controlled by #submit_route => TRUE/FALSE property on the element.

+++ b/core/lib/Drupal/Core/Render/Element/PathElement.php
@@ -0,0 +1,95 @@
+    $info['#validate_path'] = TRUE;
+    $info['#convert_path'] = self::CONVERT_ROUTE;
+    $info['#element_validate'] = array(
+      array($class, 'validateMatchedPath'),
+    );
...
+    if (!empty($element['#value']) && $element['#validate_path'] || $element['#convert_path'] != self::CONVERT_NONE) {

So now these two brand new properties are specific to a single validator, and have no default values.

That seems like a mistake.

Also, this static method and constants are on PathElement extends Textfield, meaning everyone needs to extend Textfield to get that. Could traits help us here?

@tim.plunkett: Could you clarify your concerns? Where and how do you see a need/use-case for horizontal extensibility?

More specialized implementations are covered by subclassing already. And in case that isn't sufficient for any reason, then at least the #element_validate callback can be re-used by simply specifying it in your own code (but I can't see a use-case for that).

I misread/misunderstood the change, sorry.

Though I would like to see one element/test case added when not specifying #convert_path to prove it is truly optional.

+++ b/core/modules/system/tests/src/Element/PathElementFormTest.php
@@ -0,0 +1,175 @@
+    /*$entity_converter = \Drupal::service('paramconverter.entity');
+    \Drupal::service('paramconverter_manager')->addConverter($entity_converter, 'paramconverter.entity');*/

Sorry, should have called this out before. Please remove :)

I think this is good now.

Looks good to me! Thanks.

A couple of things when using this. I have created a form with a #type => 'path' element and ran into a couple of issues:

1. I entered a path e.g. node/1, it passed the form validation. When I submitted my form, I got a "The requested page could not be found" error. There was nothing reported in the error log. I debugged and found it got a http not found exception. Once I created an actual node at that path it did work, however should it have passed validation? I guess the question is, it validates that the user has entered a 'valid' path, but should it validate if the user entered a path that exists/is accessible?
2. In my form, the element is optional. When I attempt to submit the form with no value, it fails validation stating 'There is no such path .'. Shouldnt this ignore the empty value? I tried specifically setting the #required => FALSE, but I still got the same issue.

Tested and fixed, thanks larowlan

1. +++ b/core/lib/Drupal/Core/Render/Element/PathElement.php
   @@ -0,0 +1,101 @@
   +        $form_state->setError($element, t('There is no such path %path.', array(
   ...
   +        $form_state->setError($element, t('You cannot use an external URL, please enter a relative path.'));
   ...
   +        $form_state->setError($element, t('There is no such path %path.', array(
   

   t() looks so old-skool :)

2. +++ b/core/modules/system/tests/src/Element/PathElementFormTest.php
   @@ -0,0 +1,196 @@
   +    // Valid form state.
   +    $this->assertEqual(count($form_state->getErrors()), 3);
   +  }
   

   Should we not assert what this errors actually are?

The t() is unavoidable right now, since this method is static.
Addressed #39.2

Overall this sounds very useful. But a couple of concerns:

1) We already have a #type => url for an HTML5 form input of a URL. Now we're also adding a #type => path. What's the difference? And can we just make #type => url a bit smarter so we can ditch path and don't need to explain the difference between two nearly identically-sounding form inputs? (Esp. given that "url" is a native HTML5 type and "path" is something we made up :))
2) If we do need both, it would help if there were any usages of this in core at all, so if I get confused I can at least use grep and figure out when core uses one or the other. But atm the only place this is used is in tests so that doesn't really help.

So the docs should make it clear when to use one or the other, if we have to keep both. Also:

+++ b/core/lib/Drupal/Core/Render/Element/PathElement.php
@@ -0,0 +1,101 @@
+ * Provides a matched path render element.

This is gobbledygook to me. :)

Note that #type 'url' produces a HTML5 input[type=url] element, which is validated to be a fully-qualified URL by UAs.

cf. #2054011: Implement built-in support for internal URLs (Link module/field)

rename from core/modules/system/tests/src/Element/PathElementFormTest.php
rename to core/modules/system/src/Element/PathElementFormTest.php

Hmmm, shouldn't this have been core/modules/system/tests/src/Unit/ ?

Kernel tests still extend web tests, wouldn't they still be in core/modules/system/src/Tests?

Nothing should be in core/modules/system/src/Element.

Thanks! RTBC +1

Committed ce8dcd4 and pushed to 8.0.x. Thanks!