path_form_element_validate does not send correct $name parameter to form_set_error

When we have $element available anyway, we should rather use form_error(), as in this example: http://api.drupal.org/api/drupal/developer--topics--forms_api_reference....

Also against 8.x first, then backport.

Thank you. The patch looks good and solves the problem.

You could already prepare a 7.x patch and upload it. Since it will fail the automated testing when tested in an 8.x issue, you could name it something-d7.patch, so that it will be ignored by the testbots.

We also have a "patch (to be ported)" issue status that is used when an issue has been fixed but no backport is available by then, so that no new issue must be created.

Thanks for the patch. We should write an automated test for this though before committing.

@tedbow: Should I do that, or do you want to write a test case? Do you need help there?

Have a look at:

  // Create a simpletest_example node using the node form
  public function testSimpleTestExampleCreate() {
    // Create node to edit.
    $edit = array();
    $edit['title'] = $this->randomName(8);
    $edit["body[und][0][value]"] = $this->randomName(16);
    $this->drupalPost('node/add/simpletest-example', $edit, t('Save'));
    $this->assertText(t('Simpletest Example Node Type @title has been created.', array('@title' => $edit['title'])));
  }

from the example module: http://drupalcode.org/project/examples.git/blob_plain/refs/heads/8.x-1.x....

1. It builds an $edit array of POST variables.
2. Then drupalPosts's it using the url and the name of the submit button.
3. After the POST request you can assertText to check for a text on the page and assertRaw to check for raw text, including html. There are also a lot more possible assertions: http://api.drupal.org/api/drupal/modules--simpletest--drupal_web_test_ca....

This is a real world example from the core test suite:
http://api.drupal.org/api/drupal/modules--node--node.test/function/PageE.... Click "View source" at the bottom of http://api.drupal.org/api/drupal/modules--node--node.test/class/PageEdit... for the full test case class.

Both, probably. Just the testcase, so that we see the problem actually exists - we're expecting that test to fail. And a combined patch, that should pass. The latter will get comitted, eventually.

To push this forward: Here's a test that fails without the fix and should pass with tedbow's fix.

Yeah ... it needs to be reviewed. Set it back to "needs work" when you find coding style issues or something doesn't work as expected. Someone who didn't yet work on this can also mark it as "reviewed & tested by the community" when everything looks good. That is the last step before committing.

+++ b/core/modules/path/path.testundefined
@@ -162,6 +162,23 @@ class PathTestCase extends DrupalWebTestCase {
+   * Tests that tries to create a duplicate alias are caught by validation.

I had to read this a couple times to understand it; I'd suggest:

Tests that duplicate aliases fail validation.

Looks good other than that. Thanks!

Edit: Screenshots and manual testing might be a good idea for this issue, too.

Ok, thanks for reviewing.

Doxygen reworded, these are the screenshots:

(before)

(after)

This looks great to me. (I'm assuming tests will pass; the queue is a bit backed up.) Thanks @Niklas Fiekas and @tedbow!

Great little bug fix, great little test! :)

Committed and pushed to 8.x and 7.x. Thanks!