When creating a view that displays a summary when contextual filter is not present and a base path is set, the resulting page generates a fatal error for using $this when not in an object context which comes from line 280 of views.theme.inc [$tokens = $this->getArgumentTokens()]. There are actually 4 references to "$this" in the file and I'm assuming that the others will generate the same fatal error.

CommentFileSizeAuthor
#11 summary_fatal-2685021-11.patch4.99 KBlendude
#11 summary_fatal-2685021-11-TEST_ONLY.patch3.05 KBlendude
#7 fatal_error_this_when-2685021-7.patch1.78 KBcilefen
#6 fatal_error_this_when-2685021-6.patch1.54 KBcilefen

Comments

r0nn1ef created an issue. See original summary.

cilefen’s picture

cilefen commented
Priority: Normal » Major
Issue tags: -views

Nice find. There is not a class to be seen in this file. I am bumping this to Major priority.

cilefen’s picture

cilefen commented
Version: 8.0.5 » 8.1.x-dev
cilefen’s picture

cilefen commented
Title: Fatal error: $this when not in object context » Fatal error: $this when not in object context in views.theme.inc.
r0nn1ef’s picture

r0nn1ef commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Status: Active » Needs work
Issue tags: +Needs tests
StatusFileSize
new fatal_error_this_when-2685021-6.patch1.54 KB

This is just part of it. viewsTokenReplace() is a protected method on a views plugin. This needs test coverage.

cilefen’s picture

cilefen commented
Status: Needs work » Needs review
StatusFileSize
new fatal_error_this_when-2685021-7.patch1.78 KB
r0nn1ef’s picture

r0nn1ef commented

Thanks for the quick turn around on this. I've applied the patch in #7 and no longer receive the fatal error.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Needs work

Fix looks good, setting to 'needs work' for the tests.

andypost’s picture

andypost
he/him
Primary language Russian
commented

A test view to reproduce the issue
EDIT https://gist.github.com/andypost/ccb9fbf00298f49388a4

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new summary_fatal-2685021-11-TEST_ONLY.patch3.05 KB
new summary_fatal-2685021-11.patch4.99 KB

Here are some tests for this, as the tests will show, the fix in #7 solves the fatal but the extra arguments are not added to the base path in the summary, so all links in the summary just point to the base path.

The last submitted patch, 11: summary_fatal-2685021-11-TEST_ONLY.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 11: summary_fatal-2685021-11.patch, failed testing.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
StatusFileSize
new summary_fatal-2685021-14.patch7.58 KB

No idea if this is the right way to do this, but it works and turns the test green locally.

Added more tests and redid the ones in #11

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

Suppose <front> should be a route fallback

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2685021-14-16.txt795 bytes
new summary_fatal-2685021-16.patch7.58 KB

@andypost good point, updated.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new interdiff-2685021-16-17.txt915 bytes
new summary_fatal-2685021-17.patch7.59 KB

Bleh, forgot to update the comment

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Looks ready for me

dawehner’s picture

dawehner
Primary language German
commented

I always really like if we break things, and then add test coverage due to that.

+++ b/core/modules/views/views.theme.inc
@@ -277,11 +277,30 @@ function template_preprocess_views_view_summary(&$variables) {
+      /** @var \Drupal\Core\Url $url */
       $url = Url::fromUserInput('/' . $base_path);

@@ -346,11 +366,28 @@ function template_preprocess_views_view_summary_unformatted(&$variables) {
+      /** @var \Drupal\Core\Url $url */
       $url = Url::fromUserInput('/' . $base_path);

Nitpick: I don't see why you would have to specify this type here, but nevermind.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new interdiff-2685021-17-20.txt1.24 KB
new summary_fatal-2685021-20.patch7.51 KB

Clean up per #19, no idea why I stuck that in.

dawehner’s picture

dawehner
Primary language German
commented

Okay, cool!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 6748315 and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!

  • alexpott committed 1d13c5f on 8.2.x 
    Issue #2685021 by Lendude, cilefen, r0nn1ef, andypost: Fatal error: $...

  • alexpott committed 6748315 on 8.0.x 
    Issue #2685021 by Lendude, cilefen, r0nn1ef, andypost: Fatal error: $...
lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Fixed

Uhh no idea how this got to 'needs work' again, back to fixed.

Status: Fixed » Closed (fixed)

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

lamp5’s picture

lamp5
Primary language Polish
Location Rzeszow
commented

Any plans to fix this on 8.3.x ??