Reconcile entity forms and confirm forms

overall i like this:) one more DX win!

+++ b/core/lib/Drupal/Core/Entity/EntityConfirmFormBase.phpundefined
@@ -0,0 +1,99 @@
+    if (isset($_GET['destination'])) {
+      $options = drupal_parse_url($_GET['destination']);

Since we are here..
If we are depending on Request why dont we have it as argument on buildForm() and store it in a property?
Like we did for ban module..

Unless of course its not possible for confirm forms yet?

+++ b/core/lib/Drupal/Core/Entity/EntityConfirmFormBase.phpundefined
@@ -0,0 +1,108 @@
+ * Provides an generic base class for an entity-based confirmation form.

my english are not perfect but i think its: *a* generic

+++ b/core/modules/menu/menu.moduleundefined
@@ -142,14 +142,18 @@ function menu_menu() {
-function menu_entity_info_alter(&$entity_info) {
+function menu_entity_info(&$entity_info) {

hmm i can see why you do this, but maybe there is a reason it was like that?

+++ b/core/modules/node/node.moduleundefined
@@ -1930,6 +1930,10 @@ function theme_node_recent_content($variables) {
+  if ($form_state['controller']->getOperation() == 'delete') {
+    return;
+  }

this could use a comment

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Form/BreakLockForm.phpundefined
@@ -7,18 +7,17 @@
+use Drupal\Core\Entity\EntityConfirmFormBase;
+use Drupal\Core\Entity\EntityControllerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
-
-use Drupal\Core\ControllerInterface;
-use Drupal\Core\Form\ConfirmFormBase;
-use Drupal\views\ViewStorageInterface;
 use Drupal\Core\Entity\EntityManager;
 use Drupal\user\TempStoreFactory;
+use Symfony\Component\HttpFoundation\Request;

since we are touching those lets order them alphabetically

great..only minus here is that we are duplicating logic between entity and entityNG forms, but one is to be axed anyway..so marking rtbc, i am sure bot will have no problems

great job, thanks!

the interdiff shows how many less lines you have to write now for an entity delete confirm form:)

I don't think we should change the methods from protected to public. Why not an empty interface if necessary? What's the problem with just having the protected method on the abstract class - that's what abstract classes are good at - enforcing internal implementation rather than public API.

Couple of minor nits

+++ b/core/modules/aggregator/lib/Drupal/aggregator/FeedInterface.phpundefined
@@ -14,4 +14,11 @@
+  /**
+   * Removes all items from a feed.
+   *
+   * @return self
+   */
+  public function removeItems();

The implement does not return $this and doing so would be unnecessary.

+++ b/core/modules/image/lib/Drupal/image/Form/ImageStyleDeleteForm.phpundefined
@@ -7,64 +7,46 @@
+  public function form(array $form, array &$form_state) {
+    $replacement_styles = array_diff_key(image_style_options(), array($this->entity->id() => ''));
     $form['replacement'] = array(
       '#title' => t('Replacement style'),
       '#type' => 'select',
@@ -72,16 +54,16 @@ public function buildForm(array $form, array &$form_state, ImageStyle $image_sty

@@ -72,16 +54,16 @@ public function buildForm(array $form, array &$form_state, ImageStyle $image_sty
       '#empty_option' => t('No replacement, just delete'),
     );
 
-    return parent::buildForm($form, $form_state);
+    return $form;

Talked about this with @timplunkett and he said we should be calling parent::form() here... probably before we add $form['replacement'].

These chances are fine!

Committed 73fbcf8 and pushed to 8.x. Thanks!

Need to review existing change records to see if any need an update.

There's some unpredictable behavior detected that needs follow-up #1946456-55: Convert taxonomy_term_confirm_delete() and taxonomy_vocabulary_confirm_delete() to the new form interface

I think better make a name '_confirm_form' suffix.
It works fine now but there's no ability to pass anything from container to form

Removed useless spaces.
Change notice should point about that alter should use hook_form_$entitytype_confirm_form_alter

Committed fd5f765 and pushed to 8.x. Thanks!

As per confirm_form() deprecated in favor of \Drupal\Core\Form\ConfirmFormBase form controller should extend Drupal\Core\Form\ConfirmFormBase.
If confirm form is used to provide some kind of entity function then form controller should extend Drupal\Core\Entity\EntityConfirmFormBase see class VocabularyDeleteForm for further details.
For entity confirm form routing.yml entry should look like this

taxonomy_vocabulary_delete:
  pattern: '/admin/structure/taxonomy/manage/{taxonomy_vocabulary}/delete'
  defaults:
    _entity_form: 'taxonomy_vocabulary.delete'
  requirements:
    _entity_access: 'taxonomy_vocabulary.delete'

see taxonomy.routing.yml for further details.
As this is entity form _entity_form: 'taxonomy_vocabulary.delete' we have to add form class Drupal\taxonomy\Form\VocabularyDeleteForm to entity info using @EntityType or hook_entity_info or hook_entity_info_alter.
See Core/Entity/Vocabulary.php "delete" = "Drupal\taxonomy\Form\VocabularyDeleteForm" for @EntityType entry or action_entity_info $entity_info['action']['controllers']['form']['delete'] = 'Drupal\action\Form\ActionDeleteForm'; for hook_entity_info_alter entry.

#2083415: [META] Write up all outstanding change notices before release.

Tagging "Missing change record", as completing the change record updates here blocks a beta: #2083415: [META] Write up all outstanding change notices before release

The patch for this issue was committed June 17, 2013, meaning that the change record updates for this issue have been outstanding for over seven months. Someone knowledgeable about the issue (cough @tim.plunkett) should review the draft @jibran has provided and ensure that the relevant change records get updated.

This issue needs work, either because of jibran's comment: https://drupal.org/comment/7683993#comment-7683993, if not that, then the change notice should be written. So changing status accordingly.

I added an example for EntityConfirmFormBase in https://drupal.org/node/1945416
since the regression with form ids is fixed, i dont think there is something more needed here

@balagan it was NR because in #41 I purposed the change record draft. Issue is was fixed a long ago.