Problem/Motivation

The views builds a form when it should not. If a field usually triggers views to build a form, but the format is switched to not use fields, then the form is created.

Background

DisplayPluginBase.php runs


    // If form fields were found in the view, reformat the view output as a form.
    if ($view->hasFormElements()) {

in core/modules/views/src/ViewExecutable.php, line 2450 function hasFormElements runs 


  foreach ($this->field as $field) {
    if (property_exists($field, 'views_form_callback') || method_exists($field, 'viewsForm')) {
      return TRUE;
    }
  }

Then a form is created.

How to reproduce

Reference #2855691: Display is wrap in a form and Save button is shown

Install draggableviews module then:

  1. Create a view with a display NOT using fields (e.g. rendered teasers)
  2. Create a second display that uses the table format (to be used as draggable)
    The display format will have to be set for this display only, not all displays.
    For: This page (override)
  3. Add the draggableview content field, but DON'T select to override the defaults.
    For: All displays

Without any external dependencies:
* Add a second page display to the content view
* Give it a path
* For the new display only: set the format to a grid
* For the new display only: set the show to content | teasers
* Visit the new page

Expected result:
The bulk action selector form isn't shown

Actual result:
The bulk action selector form is shown

CommentFileSizeAuthor
#15 2917239-15.patch2.78 KBLendude
#15 interdiff-2917239-12-15.txt1.18 KBLendude
#12 2917239-12.patch2.79 KBLendude
#12 2917239-12-TEST_ONLY.patch2.05 KBLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

iStryker created an issue. See original summary.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs manual testing, +Needs screenshots, +Bug Smash Initiative

Is this still an issue?

Can you provide some screenshots or a sample view?

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active
Issue tags: -Needs manual testing, -Needs screenshots

Updated the steps to reproduce with steps that have no dependencies on other things than core.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Status: Active » Needs review
FileSize
2917239-12-TEST_ONLY.patch2.05 KB
2917239-12.patch2.79 KB

Test and a fix. Since we are evaluating if $fields contains anything with a form, it makes sense to only test this if the current display actually uses fields.

The last submitted patch, 12: 2917239-12-TEST_ONLY.patch, failed testing. View results

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented

Thanks for finding this and the prompt fix! Mostly looks great. 1 tiny winy nit standing between me and RTBC (points 2 and 3):

  1. +++ b/core/modules/views/src/ViewExecutable.php
@@ -2450,9 +2450,11 @@ public function buildThemeFunctions($hook) {
-    foreach ($this->field as $field) {
-      if (method_exists($field, 'viewsForm')) {
-        return TRUE;
+    if ($this->getDisplay()->usesFields()) {
+      foreach ($this->field as $field) {
+        if (method_exists($field, 'viewsForm')) {
+          return TRUE;
+        }

    Small, obvious, clear fix for the bug. Nothing to complain about. This change is 💯 self-documenting, doesn't need a comment to see what's happening and why. 

  2. +++ b/core/modules/views/tests/modules/action_bulk_test/config/install/views.view.test_bulk_form.yml
@@ -178,3 +178,30 @@ display:
+      path: form-without-fields

    Very minor nit: form-without-fields seems like a slightly confusing path for this display. It's not a form at all (or shouldn't be). That's the point, right? How about display-without-fields or something? 

  3. +++ b/core/modules/views/tests/src/Functional/Plugin/ViewsFormTest.php
@@ -34,6 +34,12 @@ public function testFormWrapper() {
+    $this->drupalGet('form-without-fields');
+    // Ensure we have no form tag on the page.
+    $xpath = $this->cssSelect('.views-form form');

    Cuz right here is some cognitive dissonance:
    1. Get "the form".
    2. Ensure there's no form.
    🤔 huh? 😉

    But:
    1. Get "display-without-fields"
    2. Ensure there's no form.
    👍

Not gonna NW over this, and I'll ping in Slack for thoughts. I'm totally fine with this landing as-is, but I think a 7 character change would make it even better....

Thanks again!
-Derek

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
FileSize
interdiff-2917239-12-15.txt1.18 KB
2917239-15.patch2.78 KB

Thanks for the feedback, agree with both points, that makes it easier to read indeed!

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented

Sweet, thanks! Just added a notification for that bot run. When I get the email that it's green, I'll RTBC. Thanks again!

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented
Status: Needs review » Reviewed & tested by the community

🎉

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: 2917239-15.patch, failed testing. View results

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented
Status: Needs work » Reviewed & tested by the community

Random QE fail. Back to RTBC.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Queued a 10.x test run

  • larowlan committed 5888e03 on 10.0.x 
    Issue #2917239 by Lendude, dww, iStryker: Form is built when not using...
  • larowlan committed 169c75c on 9.3.x
    Issue #2917239 by Lendude, dww, iStryker: Form is built when not using...
  • larowlan committed 281129e on 9.4.x
    Issue #2917239 by Lendude, dww, iStryker: Form is built when not using...
  • larowlan committed 78d9626 on 9.5.x
    Issue #2917239 by Lendude, dww, iStryker: Form is built when not using...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 10.0.x
Backported to 9.5.x, 9.4.x and 9.3.x 😌

Thanks folks 🚀

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.