Wrong default path for page.html.twig given

Uploading the patch.

This is tricky now because we have stable and classy base themes. So the default is either in the stable theme if you have no base theme in your theme's .info.yml, or classy. I would lean towards stable as the "default" page template.

Thanks!

Thanks!

+++ b/core/includes/theme.inc
@@ -1313,7 +1313,7 @@ function template_preprocess_html(&$variables) {
- * inside "modules/system/page.html.twig". Look in there for the full list of
+ * inside "core/themes/stable/templates/layout/page.html.twig". Look in there for the full list of

This should be re-wrapped to fit within 80 characters per https://www.drupal.org/node/1354#drupal.

@stefan.korn just want to mention since this is assigned it's good to give the person assigned a day or two to work on it before jumping in.

+++ b/core/includes/theme.inc
@@ -1314,8 +1314,8 @@ function template_preprocess_html(&$variables) {
+ * inside "core/themes/stable/templates/layout/page.html.twig".

Why are we putting this path in quotes? I think we should take out the quotes.

And anyway... this whole paragraph is kind of crazy. We don't have references like this in any other template_preprocess function that I am aware of, and a lot of themes have a lot of custom templates... Why are we saying this here? We should really just take out this whole paragraph, or just say "See the page.html.twig template for list of variables." and we don't need to be specific about saying which one is the "default". They all have lists of the variables in them.

@jhodgdon
I have done according to your comment.

@stefan.korn
Apologize After creating patch only, I saw it was assigned to you.

I really think that if we put the reference in here, it should go to system module and not the stable theme.

I also really think that we should not put the long explanation in there at all. The header already says it is the preprocessing for page.html.twig, and for *most* preprocessing functions, that is enough.

But I will leave it to Cottser to decide.

Anyway, definitely no quotes!

Since Stable is the default base theme (you get this by just creating a new theme with an .info.yml and not specifying a base theme) I think it would make sense to say it's the default.

However the latest patch looks reasonable to me (I agree with @jhodgdon we probably don't need a full path) other than being a bit terse, it could use the word "a" added in there :)

+++ b/core/includes/theme.inc
@@ -1327,9 +1327,7 @@ function template_preprocess_html(&$variables) {
+ * See the page.html.twig template for list of variables.

Maybe "See the page.html.twig template for a list of variables."

how about "the list of variables" not "a"?

Yup sounds good :)

Reload the patch with documentation

+++ b/core/includes/theme.inc
@@ -1313,9 +1313,7 @@ function template_preprocess_html(&$variables) {
+ * See the page.html.twig template for list of variables.

Thanks @harsha012. I think this should say "…for the list of variables." as was discussed in the recent comments :)

added patch as per the comment

The patch looks good, lets wait for tests to finish. Although I don't think there is a reason they will fail. ;-) We can mark it RTBC after that.

Thanks.

Can someone please provide an interdiff? :)

Yes it's a small patch but practice practice practice!

@harsha012,

I was working on this.
Added interdiff.

Thanks!

Committed and pushed poolside 82296bf to 8.1.x and cc369fb to 8.0.x. Thanks!