Clean up html.html.twig markup

+1 to scripts at the bottom, but why on earth are they in a wrapper?

We all know the ideal world doesn't exist, there is going to be several scenarios where developer for one reason or another will need to add their JavaScript into the header.

This is proposition is a big limitation under my point of view.

@altrugon Whether it’s above or below, some folks are going to want/need scripts in the other spot. It’s easy to move the print statement in a theme’s html.twig, or add scripts to the head (alter or drupal_add_html_head) in preprocess. Given current best practices (see html5 boilerplate etc.) recommend scripts at the bottom, changing the default placement makes sense to me.

I still have no idea why we’d wrap them in a div though. If you need to add a script to the page from JS, can’t you just document.body.appendChild()? Even if that doesn’t work, you can add the wrapper in a custom html.tpl. I’ve rarely (never) added script tags like this, so unless I’m unusual we should not be adding wrappers like this to default core markup. Goes right against the 'start from nothing' principle.

I would second ry5n that the we should follow best practice, and place it at the end of the document.
I also agree that there is no reason to wrap it in markup, as it is not a DOM element that should or would need to be managed.

+1 - unless theres a very very very (very) good reason for having a wrapper (or even a class) it should not be there.

I'm a bit concerned about defaulting to all scripts in the footer. There are some scripts that need to go in the header, if you look at HTML5 boilerplate you'll see Modernizr in the header.

It's pretty easy to specify scripts to go into the footer without moving $scripts using the scope variable. I'd rather do it there then here.

First attempt at a patch for this template, testing out our dream mark up process.

+++ b/core/themes/bartik/bartik.themeundefined
@@ -27,6 +27,22 @@ function bartik_preprocess_html(&$variables) {
+  ¶

Whitespaces

Here is an updated patch with the whitespace mentioned in #11 removed.

Needs a reroll:

Checking patch core/includes/theme.inc...
Hunk #1 succeeded at 2571 (offset -93 lines).
Checking patch core/modules/system/templates/html.html.twig...
error: while searching for:
    {{ scripts }}
  </head>
  <body{{ attributes }}>
    <div id="skip-link">
      <a href="#main-content" class="element-invisible element-focusable">
        {{ 'Skip to main content'|t }}
      </a>

error: patch failed: core/modules/system/templates/html.html.twig:37
error: core/modules/system/templates/html.html.twig: patch does not apply
Checking patch core/themes/bartik/bartik.theme...
Checking patch core/themes/bartik/templates/html.html.twig...
Checking patch core/themes/seven/templates/html.html.twig...

Rerolled the patch to work with the latest changes

i did stumpled on a little issue if we are removing the layout classes - this is might a meta issue...

Stark & Layout
By removing those now that we don't have any other layout system than block & regions should we add layout specific classes or is this a documentation issue?
if we remove the classes the layout.css file should die as well cause it serve no purpose ?

looking at the last couple of patches:
We cant really remove the layout classes from the body stark, basically cause we have nothing atm to replace em with & we should not end up in a huge bikeshed over names etc.

As i see it we have to let stay there for now, but we can easy clean up the rest of the classname overload in body (.page-node-, page-node-2 etc) those can be placed in bartik or removed completely or added in as documentation.

Sorry guys! The patch was for #2032895: Follow-up use better technique for vertical centering.

@yannickoo does this patch belongs here ;)

rerolled that patch changed the classname to follow latest changes in core

bot get to work

This looking really good to me, @mortendk. I am so glad to see that <div id="skip-link"> and the .html gone!

Looks great! One small issue:

+++ b/core/themes/bartik/bartik.theme
@@ -27,6 +44,22 @@ function bartik_preprocess_html(&$variables) {
diff --git a/core/modules/system/templates/html.html.twig b/core/themes/bartik/templates/html.html.twig

diff --git a/core/modules/system/templates/html.html.twig b/core/themes/bartik/templates/html.html.twig
similarity index 100%
copy from core/modules/system/templates/html.html.twig
copy to core/themes/bartik/templates/html.html.twig
diff --git a/core/modules/system/templates/html.html.twig b/core/themes/seven/templates/html.html.twig

diff --git a/core/modules/system/templates/html.html.twig b/core/themes/seven/templates/html.html.twig
similarity index 100%
copy from core/modules/system/templates/html.html.twig
copy to core/themes/seven/templates/html.html.twig

Is this a typo? If we're not overriding these in the theme, they shouldn't be there (for maintainability purposes). Also, if they are added, then stage them so they are actually part of the patch too.

We are adding the "old" templates (and some preprocess) to Bartik and Seven so we don't break markup/CSS there. But this still cleans up core markup and the markup that will be inherited by any contrib themes.

As far as the copy from/to in the patch, I applied the patch locally and it worked fine and created the new files.

Here is my review of the latest patch.

1. +++ b/core/includes/theme.inc
   @@ -2516,21 +2511,6 @@ function template_preprocess_html(&$variables) {
        $variables['attributes']['class'][] = 'one-sidebar';
        $variables['attributes']['class'][] = 'sidebar-second';
      }
   -  else {
   -    $variables['attributes']['class'][] = 'no-sidebars';
   -  }
   

   Curious why one-sidebar, sidebar-second, etc. are kept but no-sidebars is removed.

2. +++ b/core/modules/system/templates/html.html.twig
   @@ -35,11 +35,9 @@
   -    <div id="skip-link">
          <a href="#main-content" class="visually-hidden focusable">
            {{ 'Skip to main content'|t }}
          </a>
   -    </div>
   

   If we're removing this, we need to fix indent of the <a> as mentioned on IRC.

3. +++ b/core/themes/bartik/bartik.theme
   @@ -11,6 +11,23 @@
     * Adds body classes if certain regions have content.
     */
    function bartik_preprocess_html(&$variables) {
   +
   +  // Add a class that tells us whether we're on the front page or not.
   +  $variables['attributes']['class'][] = $variables['is_front'] ? 'front' : 'not-front';
   

   Remove blank line at the start of the function please :)

4. +++ b/core/themes/bartik/bartik.theme
   @@ -11,6 +11,23 @@
   + if ($suggestions = theme_get_suggestions(arg(), 'page', '-')) {
   +    foreach ($suggestions as $suggestion) {
   +      if ($suggestion != 'page-front' AND $suggestion != 'page-' ) {
   +        // Add current suggestion to page classes to make it possible to theme
   +        // the page depending on the current page type (e.g. node, admin, user,
   +        // etc.) as well as more specific data like page-node-12 or page-node-edit.
   +        $variables['attributes']['class'][] = drupal_html_class($suggestion);
   +      }
   +    }
   + }
   

   I just grepped and Seven is using these classes (search for page- in style.css). So this needs to be added back in seven_preprocess_html(). I didn't see any usage of html, front/not-front, logged-in/not-logged-in or sidebar classes in Seven's CSS. For the record, I also didn't find any front/not-front or logged-in/not-logged-in classes in Bartik's CSS.

5. +++ b/core/themes/bartik/bartik.theme
   @@ -27,6 +44,22 @@ function bartik_preprocess_html(&$variables) {
   +
   +  // Add information about the number of sidebars.
   +  if (!empty($variables['page']['sidebar_first']) && !empty($variables['page']['sidebar_second'])) {
   +    $variables['attributes']['class'][] = 'two-sidebars';
   +  }
   +  elseif (!empty($variables['page']['sidebar_first'])) {
   +    $variables['attributes']['class'][] = 'one-sidebar';
   +    $variables['attributes']['class'][] = 'sidebar-first';
   +  }
   +  elseif (!empty($variables['page']['sidebar_second'])) {
   +    $variables['attributes']['class'][] = 'one-sidebar';
   +    $variables['attributes']['class'][] = 'sidebar-second';
   +  }
   +  else {
   +    $variables['attributes']['class'][] = 'no-sidebars';
   +  }
   

   Most of this (other than no-sidebars) is already being added in template_preprocess_html() so this actually results in doubled up classes on Bartik:

   <body class="one-sidebar sidebar-first front logged-in one-sidebar sidebar-first">

inspired by cottsers catch about the double classes did some more cleanup ...

sidebars classes
Removed all the sidebar info from the body classes from core!
The sidebars are just regions & keep having them in default is not optimal. If you theme is build with a sidebar structure, well fine add that into your theme
if not, well woho no more region crap in you body class name.

The sidebars are added to both seven & bartik - and for the sake of not making the world explode i have added them to stark as well.
im wondering if we even wanna have "design" in stark ? - right now i dont wanna this overcloud this patch so theres patch create a stark.theme file with the preprocess in.

I think theres a couple of wins here:
- Remove the idea of always doing a 3 column layout.
- Remove unused css in body thats not used by your theme when you dont use sidebar first-last etc.
- show how simple it is to add the into in stark.theme / bartik.theme

+++ b/core/themes/bartik/bartik.theme
@@ -11,6 +11,38 @@
+      if ($suggestion != 'page-front' AND $suggestion != 'page-' ) {

+++ b/core/themes/seven/seven.theme
@@ -47,6 +47,34 @@ function seven_preprocess_maintenance_page(&$variables) {
+      if ($suggestion != 'page-front' AND $suggestion != 'page-' ) {

"AND"?! Use &&

Cool, that's looking much better!

+++ b/core/themes/stark/stark.theme
@@ -0,0 +1,29 @@
+ * Implements hook_preprocess_HOOK() for html.tpl.php.

Please change this to say "Implements hook_preprocess_HOOK() for HTML document templates." to be in line with the changes over in #2049207: [Follow up] Replace .tpl.php with .html.twig in documentation.

Crosspost…

fixed the AND to && :)

Inspired by cottsers catch about the double classes did some more cleanup ...

NO sidebars classes
Removed all the sidebar info from the body classes from core!
The sidebars are just regions & keep having them in default is not optimal. If you theme is build with a sidebar structure, well fine add that into your theme
if not, well woho no more region crap in you body class name.

The sidebars are added to both seven & bartik - and for the sake of not making the world explode i have added them to stark as well.
im wondering if we even wanna have "design" in stark ? - right now i dont wanna this overcloud this patch so theres patch create a stark.theme file with the preprocess in.

I think theres a couple of wins here:
- Remove the idea of always doing a 3 column layout.
- Remove unused css in body thats not used by your theme when you dont use sidebar first-last etc.
- show how simple it is to add the into in stark.theme / bartik.theme

Re #34:
Nice work here, mortendk. I completely agree with these motivations that the solution here. Having less assumptions made in template_preprocess_*() is going to make our markup much cleaner.

Can we get an RTBC?

Looks good to me. Several revisions with all issues addressed. Good to go.

+++ b/core/themes/bartik/bartik.theme
@@ -11,6 +11,38 @@
+      if ($suggestion != 'page-front' && $suggestion != 'page-' ) {

Why the space after 'page-'? We should remove that ;( Sorry guys.

Nice catch @yannickoo!

+++ b/core/themes/seven/seven.theme
@@ -47,6 +47,34 @@ function seven_preprocess_maintenance_page(&$variables) {
+        // etc.) as well as more specific data like page-node-12 or page-node-edit.

Also, these comments are three characters too long, 'page-node-edit' needs to go on the next line…

We should also update the issue summary before RTBC, just showing Stark's markup before/after would be nice.

* me mumbles ... grrr .* oh well here ya go ;)

Doing the update on the issue que as well + a ton of screenshots, on a fresh install for the overlords

Less assumptions for theming in core and moving this to the themes is indeed cleaner. If you need them adding them is easy enough .

You where mentioning on irc "do we need htm.html.twig and pag.htm.twig or can we combine them".
Anybody a opinion about that?

#40 I would say, keep the html.html.twig and the page.html.twig, the thought behind this is: you put your html and head into html.html.twig so that is kind of a static file, then within that you load all your different page.html.twig files.

I think most framework work with this in mind and personally i don't want to keep track of all my html / head elements in all the different page.html.twig files (as we did in Drupal 6).

I guess that Morten will say "I give a fuck on Drupal coding standards" to me next time when we'll meet us ;)

1. +++ b/core/includes/theme.inc
   @@ -2510,40 +2510,7 @@ function template_preprocess_html(&$variables) {
   +  // This allows advanced theming based on context (node of certain type, etc.).
   

   Same here.

2. +++ b/core/themes/bartik/bartik.theme
   @@ -11,6 +11,38 @@
   +        // the page depending on the current page type (e.g. node, admin, user,
   +        // etc.) as well as more specific data like page-node-12 or page-node-edit.
   

   As mentioned above they need to be go into the next line.

3. +++ b/core/themes/seven/seven.theme
   @@ -47,6 +47,35 @@ function seven_preprocess_maintenance_page(&$variables) {
   +        // the page depending on the current page type (e.g. node, admin, user,
   

   Also this one here.

heres the screenshots of the changes including on of the source

i will ;)

lets put the html.html.twig + page.html.twig into another issue - so we can get this cleaned up first, then take that battle -> https://drupal.org/node/2090967

moving scripts to the bottom is here : https://drupal.org/node/2090991

now lets get this baby ready to fly & make frontenders happy world wide :)

fixed the 80 chars comments issue that bartik.theme had (yes not created by this patch gentlemen ;) )

added in the new summary for the issue
created a new issue for html + page combination
created a new issue to discuss the scripts in bottom of the page
screenshots for it all ...

now can i get a RTBC :)

updated the summary

Just I tought after a small discussion on IRC with "peteainsworth"

Why do we still use names like sidebar?
For example in a responsive layout the side bar drops below the main content on a phone and turns in a "Below Bar".

I think we need to think up a other name for them, like "Sub-content-first etc.

@dbazuin, cool idea. Please create a new issue for that discussion :)

I will

I added the issue 2093341

Everything works as expected without changing any CSS. Great! Just a few comment related things, after that it looks RTBC to me.

+++ b/core/includes/theme.incundefined
@@ -2488,40 +2488,7 @@ function template_preprocess_html(&$variables) {
   // Compile a list of classes that are going to be applied to the body element.

Because I know you like deleting things, we can probably loose the comments up here.

+++ b/core/themes/seven/seven.themeundefined
diff --git a/core/modules/system/templates/html.html.twig b/core/themes/seven/templates/html.html.twig
copy from core/modules/system/templates/html.html.twig

I think we need to change the comments in the theme template overrides, looking at Bartik's page.html.twig as an example:
“ * Bartik's theme implementation to display a single page.”

I will put my love in this one :)

Deleted suggested comment in core/includes/theme.inc

@LewisNeyman: Didn't understand your suggestion of changing the comments in the theme template overrides. It seems not to be the same template description and variables in core/modules/system/templates/html.html.twig and core/themes/seven/templates/html.html.twig compared to core/themes/bartik/templates/page.html.twig

As soon as you provide me more details I can finish this issue ;)

Thanks!!

This is the patch after talking with LewisNyman. Hope to have made it right :)

Nice. Thanks!

+++ b/core/themes/seven/templates/html.html.twig
--- /dev/null
+++ b/core/themes/stark/stark.theme

+++ b/core/themes/stark/stark.theme
+++ b/core/themes/stark/stark.theme
@@ -0,0 +1,29 @@

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Functions to support theming in the Stark theme.
+ */
+
+/**
+ * Implements hook_preprocess_HOOK() for HTML document templates.
+ *
+ * Adds body classes if certain regions have content.
+ */
+function stark_preprocess_html(&$variables) {
+  // Add information about the number of sidebars.
+  if (!empty($variables['page']['sidebar_first']) && !empty($variables['page']['sidebar_second'])) {
+    $variables['attributes']['class'][] = 'two-sidebars';
+  }
+  elseif (!empty($variables['page']['sidebar_first'])) {
+    $variables['attributes']['class'][] = 'one-sidebar';
+    $variables['attributes']['class'][] = 'sidebar-first';
+  }
+  elseif (!empty($variables['page']['sidebar_second'])) {
+    $variables['attributes']['class'][] = 'one-sidebar';
+    $variables['attributes']['class'][] = 'sidebar-second';
+  }
+  else {
+    $variables['attributes']['class'][] = 'no-sidebars';
+  }

Hm. I'd like JohnAlbin to take a look at this prior to commit if possible. My understanding was that Stark was intended to not add *any* extra HTML and instead just expose what core does. This seems to work against this idea.

the issue is that stark comes with a "design" - thats the only reason to add the sidebar preprocess.
core should not ship with the assumption of having a 3 column layout :)

the thing is now - do stark ship with a layout ?
if not - which i think makes sense, then let us clean that up :)

Just a idea if we give stark a layout why not give it a full width one?

First off, the patch doesn't apply because Seven doesn't have a seven_preprocess_html() any more. Looks super easy to re-roll.

1. +++ b/core/modules/system/templates/html.html.twig
   @@ -35,11 +35,9 @@
   -    <div id="skip-link">
   -      <a href="#main-content" class="visually-hidden focusable">
   -        {{ 'Skip to main content'|t }}
   -      </a>
   -    </div>
   +    <a href="#main-content" class="visually-hidden focusable">
   +      {{ 'Skip to main content'|t }}
   +    </a>
   

   I'm good with killing the div and the #skip-link ID. However, I would like to see the "skip-link" become a class on the <a> tag. The skip-link is, arguably, a design object that needs _some_ special styling on 99% of sites; if we provide the class by default, we don't have to override the default template. That would allow us to kill the html.html.twig file in both Seven and Bartik (though, we probably need to update some CSS selectors.

2. +++ b/core/themes/seven/seven.theme
   @@ -47,6 +47,35 @@ function seven_preprocess_maintenance_page(&$variables) {
    function seven_preprocess_html(&$variables) {
      drupal_add_library('system', 'normalize');
   +
   +  // Add information about the number of sidebars.
   +  if (!empty($variables['page']['sidebar_first']) && !empty($variables['page']['sidebar_second'])) {
   +    $variables['attributes']['class'][] = 'two-sidebars';
   +  }
   +  elseif (!empty($variables['page']['sidebar_first'])) {
   +    $variables['attributes']['class'][] = 'one-sidebar';
   +    $variables['attributes']['class'][] = 'sidebar-first';
   +  }
   +  elseif (!empty($variables['page']['sidebar_second'])) {
   +    $variables['attributes']['class'][] = 'one-sidebar';
   +    $variables['attributes']['class'][] = 'sidebar-second';
   +  }
   +  else {
   +    $variables['attributes']['class'][] = 'no-sidebars';
   +  }
   +
   +  if ($suggestions = theme_get_suggestions(arg(), 'page', '-')) {
   +    foreach ($suggestions as $suggestion) {
   +      if ($suggestion != 'page-front' && $suggestion != 'page-' ) {
   +        // Add current suggestion to page classes to make it possible to theme
   +        // the page depending on the current page type (e.g. node, admin, user,
   +        // etc.) as well as more specific data like page-node-12 or
   +        // page-node-edit.
   +        $variables['attributes']['class'][] = drupal_html_class($suggestion);
   +      }
   +    }
   

   I know Seven needs the sidebar classes for the installer. But do we need all those classes for the installer? Let's verify which ones it actually uses and just insert those.

   Also, the $suggestions-based classes are almost definitely not needed. Nix them.

My understanding was that Stark was intended to not add *any* extra HTML and instead just expose what core does.

Yes, that is still the reason-de-etre of Stark. It's a window into Drupal’s default markup and CSS. But there's always been one exception: Stark's layout. With responsive design, the old layout classes don't make sense for anyone's layout, except for Stark. So either we should move those classes to Stark (like the patch does) or we should delete Stark's layout completely. Stark currently has a responsive layout, so I'd lean to leaving the layout in Stark for D8, but I wouldn't oppose removing it if that's consensus.

Overall, I'm really pleased with the direction of this patch.

Patch re-rolled. These are my points about @JohnAlbin comments:

1. Added "skip-link" class to anchor element.
2. seven_preprocess_maintenance_page function doesn't exist anymore in seven.theme file.

+++ b/core/includes/theme.inc
@@ -2503,42 +2503,6 @@ function _template_preprocess_default_variables() {
-  // Compile a list of classes that are going to be applied to the body element.
-  // This allows advanced theming based on context (home page, node of certain type, etc.).
-  $variables['attributes']['class'][] = 'html';
-  // Add a class that tells us whether we're on the front page or not.
-  $variables['attributes']['class'][] = $variables['is_front'] ? 'front' : 'not-front';
-  // Add a class that tells us whether the page is viewed by an authenticated user or not.
-  $variables['attributes']['class'][] = $variables['logged_in'] ? 'logged-in' : 'not-logged-in';
...
-  // Add information about the number of sidebars.
-  if (!empty($variables['page']['sidebar_first']) && !empty($variables['page']['sidebar_second'])) {
-    $variables['attributes']['class'][] = 'two-sidebars';
-  }
-  elseif (!empty($variables['page']['sidebar_first'])) {
-    $variables['attributes']['class'][] = 'one-sidebar';
-    $variables['attributes']['class'][] = 'sidebar-first';
-  }
-  elseif (!empty($variables['page']['sidebar_second'])) {
-    $variables['attributes']['class'][] = 'one-sidebar';
-    $variables['attributes']['class'][] = 'sidebar-second';
-  }
-  else {
-    $variables['attributes']['class'][] = 'no-sidebars';
-  }
...
-  // Populate the body classes.
-  if ($suggestions = theme_get_suggestions(arg(), 'page', '-')) {
-    foreach ($suggestions as $suggestion) {
-      if ($suggestion != 'page-front') {
-        // Add current suggestion to page classes to make it possible to theme
-        // the page depending on the current page type (e.g. node, admin, user,
-        // etc.) as well as more specific data like node-12 or node-edit.
-        $variables['attributes']['class'][] = drupal_html_class($suggestion);
-      }
-    }
-  }

I don't know a single custom theme I've ever had for a client that didn't use this stuff. You're crippling contrib/custom themes with this change.

I don't personally use the sidebar classes, but would definitely miss front, not-front, logged-in, not-logged-in and the body classes.

Please update the summary. After seeing #60 and #61 -1 for me as well for this change.

Agree with tim.plunkett and dead_arm.

This still needs work to ensure there is only 1 html.html.twig file, and that all #skip-link are .skip-link.

But here is a discussed compromise from IRC.
Proof-of-concept, naming is up for debate, and it needs docs.

Interdiff was meaningless :(

1. +++ b/core/includes/theme.inc
   @@ -2634,6 +2593,61 @@ function template_preprocess_html(&$variables) {
   +  $class[] = 'html';
   

   I would name it $classes instead of $class because it returns an array with multiple classes. Otherwise see the function name ;)

2. +++ b/core/includes/theme.inc
   @@ -2634,6 +2593,61 @@ function template_preprocess_html(&$variables) {
   +  $class = array();
   

   Same here.

3. +++ b/core/includes/theme.inc
   @@ -2634,6 +2593,61 @@ function template_preprocess_html(&$variables) {
   +  $class = array();
   

   And again.

I replaced $variables['attributes']['class'][] with $class[].
I really don't care that much though.

I would prefer not to have .no-sidebars class added to body when I install a custom theme or contrib theme that already handles layout. I think layout logic should be left to the theme layer and not specified in core.

As far as .logged-in and .front < these classes will be used by people pretty commonly to do different display related things and I see no hurt in leaving them.

I agree with #68. We can do without the .no-sidebars class. It comes from the assumption that every site has a 3-column, >=960px layout. That's simply not true, especially now that mobile and responsive are becoming mainstream. Often, a sidebar isn't even a bar on the side. Usually when I need such a class, I still need to tweak the logic behind it because the default doesn't exactly match my use case.

The other two are really useful, especially .front is needed in almost every theme I make.

I know Seven needs the sidebar classes for the installer. But do we need all those classes for the installer? Let's verify which ones it actually uses and just insert those.

In Portland, we figured that the easiest way to clean up the core templates is to copy the old templates into Seven/Bartik and open up a follow up issue to clean up those templates. Otherwise we end up having to test for regressions over two/three themes with every issue. I think that's referenced in this issue.

I don't know a single custom theme I've ever had for a client that didn't use this stuff. You're crippling contrib/custom themes with this change.

If we agree that some classes are majority use case classes then we should keep them in but we shouldn't be trying to accommodate every situation, that the objective of dream markup. .front is a good example of a really common use case but Drupal shouldn't assume that every theme uses a three column layout. These classes are easy to add back in and it would be easy to create a base theme or a module that adds this stuff back in for upgrading D7 themes easily. This is the front-end equivalent of an API break so we should be careful.

Okay, based on the response after #60, here is a compromise:

1. The <div id="skip-link"> wrapping the link is removed and a skip-link class is added to the link itself
2. The region-specific classes are moved to their respective themes.
3. The node-specific classes are moved to the node.module
4. The important meta classes (logged in, front, page-*) are left in template_preprocess_html(). Remove them does not have consensus, and will hold up this issue.

Because the .skip-link class is now alongside .visually-hidden and .focusable, it has to combat their rather forcible CSS:

.visually-hidden.focusable:active,
.visually-hidden.focusable:focus {
  position: static !important;
}

The attached interdiff shows just the change needed to overcome that, the rest of the interdiff was pretty meaningless.

Looks really great! Thanks @tim.plunkett! This is indeed a very acceptable compromise. One tiny thing:

+++ b/core/modules/node/node.module
@@ -626,6 +626,16 @@ function node_is_page(EntityInterface $node) {
+ * Implements hook_preprocess_HOOK() for HTML document templates.
+ */
+function node_preprocess_html(&$variables) {
+  // If on an individual node page, add the node type to body classes.
+  if ($node = menu_get_object()) {
+    $variables['attributes']['class'][] = drupal_html_class('node-type-' . $node->getType());
+  }
+}
+
+/**

We should add a @todo here for #2091399: [META] Remove menu_get_object().

The compromise in #71 makes sense to me, and I think strikes the right balance for the various audiences. Patch looks good, too. Only one thing stands out, and it’s very minor:

+++ b/core/themes/bartik/css/style.cssundefined
@@ -315,17 +315,18 @@ ul.tips {
+a.skip-link,
+a.skip-link:link,

We don’t need to qualify the class selector with the element unless it’s resolving a specificity issue (in which case we can leave it for CSS cleanup). Otherwise this can just be `.skip-link`. Patch attached with just this change.

(As a separate issue, it seems like the visually-hidden styles need further review, since obviously needing !important here is not a good sign.)

Discussed in IRC and no @todo is fine, tests would discover it. This looks great to me, if no other objections, I'll go ahead and RTBC.

The removal of the a tag in the CSS selectors is also good, will keep this RTBC, just wait for patch to come back green.

Manually tested #74, the a qualifier was not needed for specificity.

i wont block this patch but im gonna open up a new issue so we can get rid of "page- page-foo-bar front not-front logged-in" etc.
imho They don't belong in Core! - and should only be used if you want em in your theme. to get them there is a documentation issue.

Please let us get a super super super clean drupal & make it easy to add what's needed when its needed.
But lets get this sucker in :)
im gonna create a follow up isssue to clean up core classes for realz

Committed 91d867f and pushed to 8.x. Thanks!

Followup issue for broken layout at #2117441: Stark layout broken as 'generic' classes removed from html.tpl.php

Updated issue summary.

Lets put the main content first and drop the

<a href="#main-content" class="visually-hidden focusable">
     {{ 'Skip to main content'|t }}
</a>

for dreamcode :0)

@mortendk did you open the issue for removing the other body classes, I can't find it, be great if we could remove those horrid page-suggestion classes.

Added markup changes from: https://drupal.org/node/2011578

Cleaned an improved styling.

Forget about last changes :(

Change notice added here: https://drupal.org/node/2155761

Mixed with change notice for #2011578: Markup for Stark's page.html.twig + maintenance-page.html.twig as per @alexpott suggestion in #2011578-67: Markup for Stark's page.html.twig + maintenance-page.html.twig

Thanks @rteijeiro!

This was the single worst decision in Drupal programming history. Making changes that broke somewhere between tens and hundreds of thousands of sites in the name of clean code severely damaged Drupal's reputation. Decisions like this keep businesses from trusting Drupal. As a tech, having taken over maintenance for a site that was poorly designed and not documented, it has been a bloddy frustration. The site has hundreds of modules, multiple directories, and more. Trying to discern how to patch it up after "security" updates has so far taken over thirty-six hours. It still does not work.

Why is there no clear link here to "This is how you can modify your broken themes to work after some busy-body coders thought it was fine to break Drupal's reputation mid cycle for little apparent reason"?

So frustrated.....

@shgrettenberg, the patch committed here is for Drupal 8, which is not even released yet. What "hundreds of modules, multiple directories, and more" are you talking about?

wow "This was the single worst decision in Drupal programming history"

i guess we broke a new record :P

And remember to always test Drupal upgrades on a copy of the site, never the production site ... why that comment is in a Drupal 8 thread at all is a bit baffling though.