This is a child of #1775842: [meta] Convert all variables to state and/or config systems

Original issue where this variable was introduced: #318636: Make l() themable (see comment #57)

Files: 
CommentFileSizeAuthor
#26 system-update.patch455 bytesxjm
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#25 drupal8.theme-link-config.25.patch455 bytessun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.theme-link-config.25.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#22 interdiff.txt618 bytescam8001
#21 1824920-cmi_convert-theme-link-20.patch2.17 KBcam8001
PASSED: [[SimpleTest]]: [MySQL] 49,250 pass(es).
[ View ]
#19 1824920-cmi_convert-theme-link-19.patch1.57 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] 49,253 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#17 1824920-cmi_convert_theme_link_variable-17.patch1.57 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] 48,900 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#15 1824920-cmi_convert_theme_link_variable-15.patch1.58 KBjustafish
PASSED: [[SimpleTest]]: [MySQL] 48,031 pass(es).
[ View ]
#15 interdiff.txt622 bytesjustafish
#13 1824920-cmi_convert-theme-link-variable-rebase.patch1.58 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] 46,450 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#7 1824920-cmi_convert-theme-link-variable-7.patch1.55 KBcam8001
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1824920-cmi_convert-theme-link-variable-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#7 interdiff.txt1.46 KBcam8001
#2 1824920-cmi_convert-theme-link-variable.patch1.49 KBcam8001
PASSED: [[SimpleTest]]: [MySQL] 46,242 pass(es).
[ View ]

Comments

cam8001’s picture

Title: Convert variable 'theme_link' to state system» Convert variable 'theme_link' to config system
cam8001’s picture

Component:base system» theme system
Status:Active» Needs review
StatusFileSize
new1.49 KB
PASSED: [[SimpleTest]]: [MySQL] 46,242 pass(es).
[ View ]

Here we go.

aspilicious’s picture

What does theme_link do?

cam8001’s picture

l() can either render links inline, or pass through theme_link() Rendering a link inline in l() is faster than using theme_link(). Because l() is so heavily used, using theme_link() can add a lot of overhead to page views, so by setting theme_link to FALSE you can disable theme_link entirely.

Here's the inline comment:

<?php
// Determine if rendering of the link is to be done with a theme function
// or the inline default. Inline is faster, but if the theme system has been
// loaded and a module or theme implements a preprocess or process function
// or overrides the theme_link() function, then invoke theme(). Preliminary
// benchmarks indicate that invoking theme() can slow down the l() function
// by 20% or more, and that some of the link-heavy Drupal pages spend more
// than 10% of the total page request time in the l() function.
if (!isset($use_theme) && function_exists('theme')) {
   
// Allow edge cases to prevent theme initialization and force inline link
    // rendering.
   
if (variable_get('theme_link', TRUE)) {
?>

You can see this comment for effulgentsia's numbers on this.

aspilicious’s picture

Status:Needs review» Reviewed & tested by the community

Ok for me this is good to go in that case.

catch’s picture

Status:Reviewed & tested by the community» Needs review

I'm not sure this should go in the same place as the list of enabled themes, it seems a bit mis-placed there. Bumping back to CNR.

cam8001’s picture

StatusFileSize
new1.46 KB
new1.55 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1824920-cmi_convert-theme-link-variable-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Moved into system.performance.

cam8001’s picture

Um, don't know what is up with that interdiff :/

catch’s picture

Status:Needs review» Reviewed & tested by the community

system.performance is a great idea for this one!

catch’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Configuration system

The last submitted patch, 1824920-cmi_convert-theme-link-variable-7.patch, failed testing.

cam8001’s picture

OK, will reroll.

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new1.58 KB
FAILED: [[SimpleTest]]: [MySQL] 46,450 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1824920-cmi_convert-theme-link-variable-rebase.patch, failed testing.

justafish’s picture

Status:Needs work» Needs review
StatusFileSize
new622 bytes
new1.58 KB
PASSED: [[SimpleTest]]: [MySQL] 48,031 pass(es).
[ View ]

Bumping system update number

aspilicious’s picture

Status:Needs review» Needs work

Number is way higher again! Srry!

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new1.57 KB
FAILED: [[SimpleTest]]: [MySQL] 48,900 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Rerolled against current head, bumped update number even higher.

Status:Needs review» Needs work

The last submitted patch, 1824920-cmi_convert_theme_link_variable-17.patch, failed testing.

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new1.57 KB
FAILED: [[SimpleTest]]: [MySQL] 49,253 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

The same test is failing here, but I have rerolled with a new system update in the meantime. Investigating.

Status:Needs review» Needs work

The last submitted patch, 1824920-cmi_convert-theme-link-19.patch, failed testing.

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new2.17 KB
PASSED: [[SimpleTest]]: [MySQL] 49,250 pass(es).
[ View ]

Thanks to timplunkett for the help!

cam8001’s picture

StatusFileSize
new618 bytes

Here's an interdiff of the changes to the test that was failing before.

ACF’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to go, agreed it should be in performance.

catch’s picture

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

sun’s picture

Title: Convert variable 'theme_link' to config system» [HEAD BROKEN] Convert variable 'theme_link' to config system
Category:task» bug
Priority:Normal» Critical
Status:Fixed» Reviewed & tested by the community
StatusFileSize
new455 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.theme-link-config.25.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
xjm’s picture

Assigned:cam8001» Unassigned
Status:Reviewed & tested by the community» Needs review
StatusFileSize
new455 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Missed an update hook addition.

xjm’s picture

lol. Crosspost.

catch’s picture

Category:bug» task
Priority:Critical» Normal
Status:Needs review» Fixed

Whoops. I credited both of yer.

xjm’s picture

Title:[HEAD BROKEN] Convert variable 'theme_link' to config system» Convert variable 'theme_link' to config system

Thanks!

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

Anonymous’s picture

Issue summary:View changes

was linking to wrong meta issue