Remove usage of menu_test_theme_page_callback() [#2563801]

Problem/Motivation

menu_test_theme_page_callback() is marked as @deprecated.

Proposed resolution

Remove the usages of menu_test_theme_page_callback() as well as the function itself.

Remaining tasks

User interface changes

API changes

Data model changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Bug because `@deprecated` code is buggy.
Issue priority	Normal because the code is functional.
Prioritized changes	Prioritized because we are removing a `@deprecated` function.
Disruption	Totally not disruptive because it's not widely called.

Comment	File	Size	Author
#18	interdiff-16-18.txt	496 bytes	sdstyles
#18	2563801-18.patch	4.69 KB	sdstyles
#18	8.0.x: PHP 5.5 & MySQL 5.5 14,101 pass
#2	remove-usage-menu-test-page-callback-2563801-1.patch	1.44 KB	rashid_786
#2	8.0.x: PHP 5.5 & MySQL 5.5 13,714 pass
#5	remove-usage-menu-test-page-callback-2563801-5.patch	1.42 KB	rashid_786
#5	8.0.x: PHP 5.5 & MySQL 5.5 13,776 pass
#7	interdiff-2563801-5-7.txt	1.29 KB	rashid_786
#7	2563801-7.patch	1.44 KB	rashid_786
#7	8.0.x: PHP 5.5 & MySQL 5.5 13,853 pass
#10	2563801-10.patch	1.43 KB	keopx
#10	8.0.x: PHP 5.5 & MySQL 5.5 14,060 pass
#10	interdiff_2563801-7-10.txt	1.22 KB	keopx
#15	2563801-15.patch	3.27 KB	joshi.rohit100
#15	8.0.x: PHP 5.5 & MySQL 5.5 14,044 pass
#15	interdiff-15.txt	2.97 KB	joshi.rohit100
#16	2563801-16.patch	4.72 KB	sdstyles
#16	8.0.x: PHP 5.5 & MySQL 5.5 14,044 pass
#16	interdiff-15-16.txt	1.3 KB	sdstyles

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

5 September 2015 at 19:15

Mile23 created an issue. See original summary.

Comment #2

rashid_786 CreditAttribution: rashid_786 as a volunteer commented 7 September 2015 at 04:28

File	Size
remove-usage-menu-test-page-callback-2563801-1.patch	1.44 KB
8.0.x: PHP 5.5 & MySQL 5.5 13,714 pass

filed an issue also to remove the callback itself #2564161.

Comment #3

rashid_786 CreditAttribution: rashid_786 as a volunteer commented 7 September 2015 at 04:28

Status:

Active

» Needs review

Comment #4

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 11 September 2015 at 02:29

Status:	Needs review	» Needs work
Related issues:		+#2564161: remove @deprecated menu_test_theme_page_callback()

Thanks. :-)

A couple things:

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,26 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   * @param bool $inherited
+   *   (optional) TRUE when the requested page is intended to inherit
+   *   the theme of its parent.
...
   public function themePage($inherited) {

$inherited is documented as being optional, but isn't optional in the code. We should follow the code for this refactoring, so change the docs so $inherited isn't optional.

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,26 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   *   for the current page request. ¶

Adds whitespace.

Comment #5

rashid_786 CreditAttribution: rashid_786 as a volunteer commented 11 September 2015 at 04:24

Status:

Needs work

» Needs review

File	Size
remove-usage-menu-test-page-callback-2563801-5.patch	1.42 KB
8.0.x: PHP 5.5 & MySQL 5.5 13,776 pass

updated patch with suggested changes.

Comment #6

NikhilValand CreditAttribution: NikhilValand commented 11 September 2015 at 05:12

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,26 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   * @return string
...
-    return menu_test_theme_page_callback($inherited);
...
+    // Now we check what the theme negotiator service returns.

In general, all lines of code should not be longer than 80 characters.
https://www.drupal.org/coding-standards#linelength

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,26 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   *   A string describing the requested custom theme and actual theme being used

In general, all lines of code should not be longer than 80 characters.
https://www.drupal.org/coding-standards#linelength

Comment #7

rashid_786 CreditAttribution: rashid_786 as a volunteer commented 16 September 2015 at 03:17

File	Size
interdiff-2563801-5-7.txt	1.29 KB
2563801-7.patch	1.44 KB
8.0.x: PHP 5.5 & MySQL 5.5 13,853 pass

line length has been reduced less than 80 characters.

Comment #8

lachezar.valchev CreditAttribution: lachezar.valchev at FFW commented 25 September 2015 at 11:45

Hi all,

I will make a review on this today :)

BR.

Comment #9

lachezar.valchev CreditAttribution: lachezar.valchev at FFW commented 25 September 2015 at 13:17

Status:

Needs review

» Needs work

Hi,

The patch applies nice on the latest 8.0.x-dev.

However, when run it through the Coder, there are a couple of errors listed below.

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,28 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   * Page callback: Tests the theme negotiation functionality.

Doc comment short description must be on the first line.

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,28 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+   *   A string describing the requested custom theme and actual ¶

Whitespace found at end of line.

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,28 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+    ->determineActiveTheme(\Drupal::routeMatch());

Line indented incorrectly; expected 6 spaces, found 4.

Comment #10

keopx

Basque

Basque Country

CreditAttribution: keopx at Isarea commented 26 September 2015 at 10:49

Status:

Needs work

» Needs review

File	Size
2563801-10.patch	1.43 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,060 pass
interdiff_2563801-7-10.txt	1.22 KB

Changes applied

Comment #11

oenie CreditAttribution: oenie commented 26 September 2015 at 12:38

Status:

Needs review

» Reviewed & tested by the community

Looks ok to me, RTBC ?

Comment #12

lachezar.valchev CreditAttribution: lachezar.valchev at FFW commented 27 September 2015 at 10:08

Hi,

Patch applies nicely and looks good to me as well.

Regards,
Lachezar

Comment #13

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 28 September 2015 at 07:12

Status:

Reviewed & tested by the community

» Needs work

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -40,10 +40,27 @@ public function titleCallback(array $_title_arguments = array(), $_title = '') {
+    $theme_key = \Drupal::theme()->getActiveTheme()->getName();
...
+    $active_theme = \Drupal::service('theme.negotiator')

These are services - we should be injecting them into the controller properly rather than using \Drupal.

If MenuTestController extended ControllerBase this would be simply a matter of implementing a create method.

Comment #14

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 28 September 2015 at 07:13

Also given this is a test method I think removal of menu_test_theme_page_callback() in this issue is fine too. It is test code.

Comment #15

joshi.rohit100

he/him

Hindi

Delhi, India

CreditAttribution: joshi.rohit100 commented 29 September 2015 at 06:24

Status:

Needs work

» Needs review

File	Size
2563801-15.patch	3.27 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,044 pass
interdiff-15.txt	2.97 KB

Comment #16

sdstyles CreditAttribution: sdstyles at FFW commented 29 September 2015 at 07:32

File	Size
interdiff-15-16.txt	1.3 KB
2563801-16.patch	4.72 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,044 pass

Also according to #14 removed menu_test_theme_page_callback()

Comment #17

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 30 September 2015 at 00:10

Issue summary:	View changes
Status:	Needs review	» Needs work

Looks great, except for one thing:

+++ b/core/modules/system/tests/modules/menu_test/src/Controller/MenuTestController.php
@@ -7,12 +7,66 @@
+  }
+
+    /**
    * Some known placeholder content which can be used for testing.
    *

Improper docblock alignment.

Sooooooo close. :-)

Comment #18

sdstyles CreditAttribution: sdstyles at FFW commented 30 September 2015 at 05:55

File	Size
2563801-18.patch	4.69 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,101 pass
interdiff-16-18.txt	496 bytes

Fixed.

Comment #19

sdstyles CreditAttribution: sdstyles at FFW commented 30 September 2015 at 05:56

Status:

Needs work

» Needs review

Comment #20

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 30 September 2015 at 06:24

Status:

Needs review

» Reviewed & tested by the community

Diggit. :-)

RTBC unless it fails.

Comment #21

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Chapter Three commented 30 September 2015 at 14:03

Status:

Reviewed & tested by the community

» Fixed

Committed 17a9dac and pushed to 8.0.x. Thanks!

Comment #22

30 September 2015 at 14:04

alexpott committed 17a9dac on 8.0.x

Issue #2563801 by rashid_786, sdstyles, keopx, joshi.rohit100, Mile23,...

Comment #23

1 October 2015 at 01:07

Status:

Fixed

» Needs work

The last submitted patch, 18: 2563801-18.patch, failed testing.

Comment #24

andypost

he/him

Russian

CreditAttribution: andypost as a volunteer and at Skilld commented 1 October 2015 at 10:19

Status:

Needs work

» Fixed

Comment #25

15 October 2015 at 10:24

Status:

Fixed

» Closed (fixed)

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

Remove usage of menu_test_theme_page_callback()

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Beta phase evaluation

Comments

Child issues

Related issues