Delete multiple for nodes is broken

This seemed to be not the only place.

Beautiful. Great work @pcambra!

Nice test coverage!

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/BulkFormTest.phpundefined
@@ -40,16 +40,85 @@ public function testBulkForm() {
+    $this->assertFalse($node->status, 'Node has been unpublished');

Shouldn't we use the EntityNG way of accessing the data?

Works for me.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/BulkFormTest.phpundefined
@@ -40,16 +40,85 @@ public function testBulkForm() {
+    $this->assertTrue($node->isSticky() == NODE_NOT_STICKY);

This shouldn't need the == ... part. isSticky() returns boolean, this only works because of the == condition. Same for a bunch of those isSticky() and isPromoted() below.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/BulkFormTest.phpundefined
@@ -40,16 +40,85 @@ public function testBulkForm() {
+    $node = entity_load('node', $node->id(), TRUE);
+    $this->assertTrue(empty($node), 'Node has been deleted');

I think an assertNull($node) makes more sense here, checking a node is "empty" feels strange.

Also, definition nitpick.. using methods has nothing to do with NG, the whole point of it is to hide that there's NG below it ;)

Thank you berdir for the review.

The points of berdir seems to be adressed.

+++ b/core/modules/node/lib/Drupal/node/Tests/Views/BulkFormTest.phpundefined
@@ -40,16 +40,85 @@ public function testBulkForm() {
+    $this->assertTrue($node->isPublished());
...
+    $this->assertTrue($node->isPublished());

I think we need a message here as the default message is not going to be helpful. I think the message will be "Value FALSE is TRUE." :)

Back to RTBC

This fix looks good for this one page, and the tests look great. Although, it looks like the root problem derives from the fact that the Request parameter on ConfirmFormBase::buildForm() is optional, despite the fact that it's actually not:

public function buildForm(array $form, array &$form_state, Request $request = NULL) {
    $path = $this->getCancelPath();
    // Prepare cancel link.
    if ($request->query->has('destination')) { # <-- Uh-oh!
      $options = drupal_parse_url($request->query->get('destination'));
    }

...so when return parent::buildForm($form, $form_state); happens, boom.

So let's get a follow-up filed to either remove the "= NULL" part of that function signature, or else actually make it optional.

In the meantime, committed and pushed to 8.x. Thanks!

Opened #2054991: Forms often require the request, but buildForm() does not enforce that as a follow-up.

I had he same problem but the patch drupal-2049569-14.patch didn't work for me (it generated an error) so i add a little modification and now it works fine !

Hope it can help.

I had he same problem but the patch drupal-2049569-14.patch didn't work for me (it generated an error) so i add a little modification and now it works fine !

Hope it can help.

@tonytroy Please create a new issue. Describe the new bug and post your patch there. After #16 the patch is the part of drupal core now you have to post the new changes in new issue. Thank you for your help.