The hook_page_top() implementation for the node module has a wrong definition. The function argument should be called $variables, not $page. And the node_preview form container should be added directly into the argument, not into a 'page_top' sub-key.

CommentFileSizeAuthor
#24 reroll_diff_19-24.txt1.26 KBsahil.goyal
#24 2832074-24.patch1.27 KBsahil.goyal
#21 interdiff_20-21.txt337 bytesTanuj.
#21 2832074-21.patch1.09 KBTanuj.
#20 drupal-fix_node_page_top_hook_implementation-2832074-20.patch1.28 KBMedha Kumari
#19 drupal-fix_node_page_top_hook_implementation-2832074-19.patch1.09 KBleolandotan
#7 drupal-fix_node_page_top_hook_implementation-2832074-7.patch1.2 KBSweetchuck
#2 drupal-fix_node_page_top_hook_implementation-2832074-2.patch1.09 KBSchnWalter
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SchnWalter created an issue. See original summary.

SchnWalter’s picture

SchnWalter CreditAttribution: SchnWalter as a volunteer and commented
Status: Active » Needs review
FileSize
drupal-fix_node_page_top_hook_implementation-2832074-2.patch1.09 KB

This patch might break some alter hooks that rely on the `node_preview` form container being in the wrong place.

Chi’s picture

Chi CreditAttribution: Chi commented

The function argument should be called $variables, not $page.

The hook definition suggests $page_top.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

Needs work because of #3.

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.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Sweetchuck’s picture

Sweetchuck
Primary language Hungarian
Location Budapest
CreditAttribution: Sweetchuck at Cheppers commented
Status: Needs work » Needs review
Issue tags: +Drupalaton 2017
FileSize
drupal-fix_node_page_top_hook_implementation-2832074-7.patch1.2 KB
amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

Looks like a good small cleanup :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: drupal-fix_node_page_top_hook_implementation-2832074-7.patch, failed testing. View results

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.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: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

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

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.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.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

DanielVeza’s picture

DanielVeza
he/him
Location Brisbane, AU
CreditAttribution: DanielVeza at PreviousNext commented
Issue tags: +Bug Smash Initiative, +Needs reroll

Same issue still exists on 10.1.x, the patch will just need to be rerolled and tested against 10.1 to make sure it still passes tests.

I think \Drupal\node\Form\NodePreviewForm can be replaced with NodePreviewForm::class, but I'm unsure if thats out of scope for this.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan at Merkle commented
FileSize
drupal-fix_node_page_top_hook_implementation-2832074-19.patch1.09 KB

I have rerolled the patch for 10.1.x but didn't include the replacement of \Drupal\node\Form\NodePreviewForm
with NodePreviewForm::class yet. I also didn't remove the "Needs reroll" tag since it would still need it for 9.5.x. I hope this is alright.

Medha Kumari’s picture

Medha Kumari
Location Jaipur
CreditAttribution: Medha Kumari as a volunteer and at Dotsquares Ltd. commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
drupal-fix_node_page_top_hook_implementation-2832074-20.patch1.28 KB

Rerolled the patch #19 in drupal 9.5.x.

Tanuj.’s picture

Tanuj. CreditAttribution: Tanuj. as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
FileSize
2832074-21.patch1.09 KB
interdiff_20-21.txt337 bytes

Fixed CCF on #20
added interdiff text

Status: Needs review » Needs work

The last submitted patch, 21: 2832074-21.patch, failed testing. View results

sahil.goyal’s picture

sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » sahil.goyal

work on this

sahil.goyal’s picture

sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: sahil.goyal » Unassigned
FileSize
2832074-24.patch1.27 KB
reroll_diff_19-24.txt1.26 KB

Reroll patch again for the version 9.5.x considering #18 and attaching the reroll_diff corresponding to #19

sahil.goyal’s picture

sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
ameymudras’s picture

ameymudras CreditAttribution: ameymudras at Salsa Digital commented
Status: Needs review » Reviewed & tested by the community

Tested on 9.5.x patch #24

1. The issue summary is clear and explains the problem
2. The code change is as expected and as described in the issue summary
3. Applied the patch and checked for the "back to content editing" link on the node preview, which is shown as expected
4. Additional tests are not required in this case

Looks RTBC to me.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, thanks!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

  • catch committed 8df5eabf on 10.1.x 
    Issue #2832074 by sahil.goyal, TanujJain-TJ, SchnWalter, Sweetchuck,...

Status: Fixed » Closed (fixed)

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