Teaser was replaced by view mode a long time ago, but was left in for some reason.

Author name, author attributes and date should depend on display_submitted but are generated even if that's not set, that's a very small performance issue.

Author picture already depends on display_submitted but this isn't documented in the template.

Also removed some other stale documentation relating to variables that are long gone like readmore.

Files: 
CommentFileSizeAuthor
#44 remove_some_pointless-2370145-44.patch11.57 KBdavidhernandez
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,584 pass(es), 10 fail(s), and 0 exception(s). View
#32 interdiff.txt1.04 KBjoelpittet
#32 remove_some_pointless-2370145-32.patch11.86 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 103,179 pass(es), 11 fail(s), and 0 exception(s). View
#29 interdiff.txt621 byteskgoel
#29 2370145-29.patch11.52 KBkgoel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,397 pass(es), 4 fail(s), and 0 exception(s). View
#22 interdiff.txt2.07 KBkgoel
#22 2370145-22.patch11.23 KBkgoel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,364 pass(es), 7 fail(s), and 0 exception(s). View
#18 remove_some_pointless-2370145-18.patch10.48 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,589 pass(es), 2 fail(s), and 0 exception(s). View
#18 interdiff.txt7.75 KBjoelpittet
#15 remove_some_pointless-2370145-15.patch9.74 KBjoelpittet
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,595 pass(es), 2 fail(s), and 0 exception(s). View
#12 remove_some_pointless-2370145-12.patch9.75 KBlauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,877 pass(es), 2 fail(s), and 0 exception(s). View
#9 node_preprocess-2370145-9.patch9.4 KBdotsi
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,544 pass(es), 6 fail(s), and 0 exception(s). View
#5 node_preprocess-2370145-5-interdiff.txt1.09 KBBerdir
#5 node_preprocess-2370145-5.patch9.55 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch node_preprocess-2370145-5.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

Status: Needs review » Needs work

The last submitted patch, node_preprocess.patch, failed testing.

lauriii’s picture

Why are we removing documentation for variables that still exists?

Berdir’s picture

Status: Needs work » Needs review
FileSize
8.46 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,062 pass(es), 6 fail(s), and 0 exception(s). View
6.02 KB

Made a few improvements, fixed bartik_preprocess_node(), and made sure that uid/created are always hidden. And updated node.html.twig in bartik.

@laurii: What are we removing that still exists? teaser no longer exists, the others are just moved.

Status: Needs review » Needs work

The last submitted patch, 3: node_preprocess-2370145-3.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
9.55 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch node_preprocess-2370145-5.patch. Unable to apply patch. See the log in the details link for more information. View
1.09 KB

Fixed that test. Switched to test both page and article to test different settings, only those that show the author now add the relevant user tags.

lauriii’s picture

I think I misread the patch because there's too many spaces before date and authorname on the documentation

Status: Needs review » Needs work

The last submitted patch, 5: node_preprocess-2370145-5.patch, failed testing.

dotsi’s picture

Status: Needs work » Needs review
FileSize
9.4 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,544 pass(es), 6 fail(s), and 0 exception(s). View

Rerolled the patch from #5. There were some conflicts.

Status: Needs review » Needs work

The last submitted patch, 9: node_preprocess-2370145-9.patch, failed testing.

Cottser’s picture

Issue tags: +Needs reroll

Needs another reroll I suppose :)

lauriii’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
9.75 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 89,877 pass(es), 2 fail(s), and 0 exception(s). View

Reroll

Status: Needs review » Needs work

The last submitted patch, 12: remove_some_pointless-2370145-12.patch, failed testing.

Cottser’s picture

Issue tags: +Needs reroll

That time again apparently. Needs reroll.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
9.74 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,595 pass(es), 2 fail(s), and 0 exception(s). View

Here's a re-roll it fell back to 3-way auto merge so nothing to resolve.

joelpittet’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/node/node.module
    @@ -591,18 +586,14 @@ function template_preprocess_node(&$variables) {
    -  // Helpful $content variable for templates.
    -  $variables += array('content' => array());
    
    @@ -610,6 +601,14 @@ function template_preprocess_node(&$variables) {
    +  // Helpful $content variable for templates.
    +  $variables += array('content' => array());
    

    Is moving this hunk needed?

  2. +++ b/core/modules/node/node.module
    @@ -591,18 +586,14 @@ function template_preprocess_node(&$variables) {
    +    $variables['date'] = drupal_render($variables['elements']['created']);
    +    $variables['author_name'] = drupal_render($variables['elements']['uid']);
    

    We don't need to drupal_render these variables, let the template do that.

  3. +++ b/core/modules/node/templates/node.html.twig
    @@ -15,13 +15,16 @@
    - * - author_picture: The node author user entity, rendered using the "compact"
    - *   view mode.
    ...
    - * - date: Themed creation date field.
    - * - author_name: Themed author name field.
    ...
    + *   If display_submitted is set, additional variables are prepared:
    
    @@ -42,17 +45,15 @@
    - * - author_attributes: Same as attributes, except applied to the author of
    - *   the node tag that appears in the template.
    ...
    - * - teaser: Flag for the teaser state. Will be true if view_mode is 'teaser'.
    

    Because of the duplication we have for templates in core from classy this also needs to happen in classy's node template.

  4. +++ b/core/modules/node/templates/node.html.twig
    @@ -15,13 +15,16 @@
    + *    - author_picture: The node author user entity, rendered using the "compact"
    ...
    + *    -  date: Themed creation date field.
    + *    -  author_name: Themed author name field.
    

    There is some coding standards for the indent and 80 chars from what dreditor is saying.

  5. +++ b/core/modules/node/templates/node.html.twig
    @@ -42,17 +45,15 @@
    + * - is_front: Flag for front. Will be true when presented on the front page.
    
    +++ b/core/themes/bartik/templates/node.html.twig
    @@ -42,17 +45,15 @@
    + * - is_front: Flag for front. Will be true when presented on the front page.
    

    This looks like it snuck in and may not exist? Or is this a clean-up of something that is missing?

  6. +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php
    @@ -97,7 +98,6 @@ function testPageCacheTags() {
    -      'user:' . $author_1->id(),
    

    Does the user: need to be removed?

  7. +++ b/core/themes/bartik/templates/node.html.twig
    @@ -1,7 +1,7 @@
    - * Bartik's theme implementation to display a node.
    + * Default theme implementation to display a node.
    

    That change is not needed as it is Bartik's.

The last submitted patch, 15: remove_some_pointless-2370145-15.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
7.75 KB
10.48 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,589 pass(es), 2 fail(s), and 0 exception(s). View

Ok I think the drupal_render() is needed... but it really shouldn't be, RDF is concatinating it and using SafeMarkup... would be nicer if they just did prefix/suffix on the render element or something. That should be a follow-up.

I've put the other bits in place and fixed the failing test but turning on display_submitted (NodeTestBase has it off be default).

Let's see if testbot agrees.

Status: Needs review » Needs work

The last submitted patch, 18: remove_some_pointless-2370145-18.patch, failed testing.

joelpittet’s picture

Hmm 2 different errors, text showing that shouldn't be...

kgoel’s picture

Assigned: Unassigned » kgoel

Looking into fails and working on it.

kgoel’s picture

Status: Needs work » Needs review
FileSize
11.23 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,364 pass(es), 7 fail(s), and 0 exception(s). View
2.07 KB
kgoel’s picture

+++ b/core/modules/node/node.module
@@ -610,6 +601,14 @@ function template_preprocess_node(&$variables) {
+  // Helpful $content variable for templates.
+  $variables += array('content' => array());
+  foreach (Element::children($variables['elements']) as $key) {
+    $variables['content'][$key] = $variables['elements'][$key];
+  }

Moved this hunk down as the test was failing because author_name and date variable should not be included in $variables['content'] . we render them before building the 'content' array so they don't show up there.
In #18 patch $variable['date'] and $variable['author_name'] was set after building the content variable.

+++ b/core/modules/node/node.module
@@ -591,18 +586,14 @@ function template_preprocess_node(&$variables) {
+    $variables['date'] = \Drupal::service('renderer')->render($variables['elements']['created']);

Replaced deprecated drupal_render with \Drupal::service('renderer')->render.

kgoel’s picture

Assigned: kgoel » Unassigned

I am going to try do some profiling tomorrow.

Status: Needs review » Needs work

The last submitted patch, 22: 2370145-22.patch, failed testing.

kgoel’s picture

Here is profiling result -

=== 8.0.x..8.0.x compared (556d0efcc18be..556d13edd0b22):

ct : 610,204|610,204|0|0.0%
wt : 2,045,741|2,013,482|-32,259|-1.6%
mu : 58,569,312|58,569,344|32|0.0%
pmu : 59,801,216|59,801,624|408|0.0%

=== 8.0.x..2370145-a compared (556d0efcc18be..556d153c4efc5):

ct : 610,204|586,877|-23,327|-3.8%
wt : 2,045,741|1,773,106|-272,635|-13.3%
mu : 58,569,312|58,496,288|-73,024|-0.1%
pmu : 59,801,216|59,727,592|-73,624|-0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=556d0efcc18be&...

Cottser’s picture

@kgoel you should include the lionsad.de links those are very important for being able to verify your results :)

joelpittet’s picture

Nice you fixed my other bug.

+++ b/core/modules/node/node.module
@@ -572,14 +572,9 @@ function node_theme_suggestions_node(array $variables) {
-  $variables['date'] = drupal_render($variables['elements']['created']);
-  unset($variables['elements']['created']);
-  $variables['author_name'] = drupal_render($variables['elements']['uid']);
-  unset($variables['elements']['uid']);

@@ -591,18 +586,14 @@ function template_preprocess_node(&$variables) {
   if ($variables['display_submitted']) {
+    $variables['author_attributes'] = new Attribute();
+    $variables['date'] = \Drupal::service('renderer')->render($variables['elements']['created']);
+    $variables['author_name'] = \Drupal::service('renderer')->render($variables['elements']['uid']);

The cache tags are no longer needed for user because by default the tests have display_submitted false and the now won't get rendered and there for don't get tags for user context bubbling up.

Just need to remove the unneeded user cache tags from the test now.

kgoel’s picture

Status: Needs work » Needs review
FileSize
11.52 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,397 pass(es), 4 fail(s), and 0 exception(s). View
621 bytes

Status: Needs review » Needs work

The last submitted patch, 29: 2370145-29.patch, failed testing.

joelpittet’s picture

Looks you are on the right track @kgoel. The next one looks like it needs timezone cache key removed.

joelpittet’s picture

FileSize
11.86 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 103,179 pass(es), 11 fail(s), and 0 exception(s). View
1.04 KB

Maybe this will finish them off? Didn't work locally (10 fails) running

php ./core/scripts/run-tests.sh  --url http://d8.dev --color --verbose --class "Drupal\page_cache\Tests\PageCacheTagsIntegrationTest"
joelpittet’s picture

Status: Needs work » Needs review

engage!

Status: Needs review » Needs work

The last submitted patch, 32: remove_some_pointless-2370145-32.patch, failed testing.

The last submitted patch, 32: remove_some_pointless-2370145-32.patch, failed testing.

The last submitted patch, 32: remove_some_pointless-2370145-32.patch, failed testing.

joelpittet’s picture

Well those 10 fails are there! Well I'll be. That's an unexpected but nice to see that local is showing the same.

davidhernandez’s picture

Assuming it needs a reroll at this point, but lets check.

The last submitted patch, 32: remove_some_pointless-2370145-32.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review

All cache failures it looks like.

davidhernandez’s picture

FileSize
11.57 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,584 pass(es), 10 fail(s), and 0 exception(s). View

Needed rerolling. Lets see if that worked.

Status: Needs review » Needs work

The last submitted patch, 44: remove_some_pointless-2370145-44.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.