Problem/Motivation

Theres no need for the div inside of forms - Drupal8 is html5 and not html 4

Proposed resolution

change form.html.twig :
From

<form{{ attributes }}><div>{{ children }}</div></form>

to

<form{{ attributes }}>
  {{ children }}
</form>

Remaining tasks

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#3 form-div-2250381-4.patch1.2 KBmortendk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,696 pass(es). View
form-inner-div.patch394 bytesmortendk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,617 pass(es), 1 fail(s), and 0 exception(s). View

Comments

ParisLiakos’s picture

Status: Active » Needs review

yes plz:)

Status: Needs review » Needs work

The last submitted patch, form-inner-div.patch, failed testing.

mortendk’s picture

Status: Needs work » Needs review
FileSize
1.2 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,696 pass(es). View

fixed the test

dawehner’s picture

What a nice and simple patch!

$ ag "form div"

Couldn't find any used css for that

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

moe

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

  • Commit a0f81c7 on 8.x by catch:
    Issue #2250381 by mortendk: Remove the inner div wrapper from forms.
    
tim.plunkett’s picture

Did anyone check for when that div was added and why?
I thought it was needed for AJAX stuff, but it goes back to at least 4.7

As far as I can see that was removed with regard to CSS, but had no manual testing or thought for JS.

tim.plunkett’s picture

From ajax.js, Drupal.AjaxCommands.prototype.insert:


      // We don't know what response.data contains: it might be a string of text
      // without HTML, so don't rely on jQuery correctly iterpreting
      // $(response.data) as new HTML rather than a CSS selector. Also, if
      // response.data contains top-level text nodes, they get lost with either
      // $(response.data) or $('<div></div>').replaceWith(response.data).
      var new_content_wrapped = $('<div></div>').html(response.data);
      var new_content = new_content_wrapped.contents();

      // For legacy reasons, the effects processing code assumes that new_content
      // consists of a single top-level element. Also, it has not been
      // sufficiently tested whether attachBehaviors() can be successfully called
      // with a context object that includes top-level text nodes. However, to
      // give developers full control of the HTML appearing in the page, and to
      // enable Ajax content to be inserted in places where DIV elements are not
      // allowed (e.g., within TABLE, TR, and SPAN parents), we check if the new
      // content satisfies the requirement of a single top-level element, and
      // only use the container DIV created above when it doesn't. For more
      // information, please see http://drupal.org/node/736066.
      if (new_content.length !== 1 || new_content.get(0).nodeType !== 1) {
        new_content = new_content_wrapped;
      }
joelpittet’s picture

I did, the reason it was in there was for XHTML compliance. Going to look for issue... brb

joelpittet’s picture

I think this was the reference I was thinking about.
https://drupal.org/node/1822210

Which @jwilson3 pointed out in #1822210-7: Investigate removing the inner div in form.html.twig

In plain english, the only valid elements inside the FORM tag include:

NOSCRIPT, INS, DEL, SCRIPT, P, H1-6, DIV, UL/OL/DL, PRE, HR, BLOCKQUOTE, ADDRESS, FIELDSET, TABLE.

and, INPUT is not a tag in that list.

And a previous removal attempt here: #495480: Add class to wrapper div of form elements theme_form()

@tim.plunkett I can't see the correlation in that massive ajax comment to removing a div from form, can you break out the part that relates to this?

mortendk’s picture

oooh that was my first real try to help out with the markup in drupalcore *gets all sobby*
Anyhow i seems to fail in my javascript & prototype.insert knowledge of to get why a form div should be there?

Status: Fixed » Closed (fixed)

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

tomogden’s picture

I would love to see this backported to D7.

joelpittet’s picture

@tomogden me too, but I doubt that would happen as people may have styled against that or worse written JS DOM traversal.

Though you can do what I do, override the theme function:


/**
 * Implements theme_form().
 */
function YOURTHEME_form($variables) {
  $element = $variables['element'];
  if (isset($element['#action'])) {
    $element['#attributes']['action'] = drupal_strip_dangerous_protocols($element['#action']);
  }
  element_set_attributes($element, array('method', 'id'));
  if (empty($element['#attributes']['accept-charset'])) {
    $element['#attributes']['accept-charset'] = "UTF-8";
  }
  // Anonymous DIV to satisfy XHTML compliance. (REMOVED)
  return '' . $element['#children'] . '';
}

And maybe @mortendk will be bold and add it to his base theme;)