Page MenuHomePhabricator

The Echo extension personalUrl hook should use a string and not an array for css class
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce:

I installed The Echo extension

Actual Results:

I get a notice about array to string conversion in includes/skins/BaseTemplate.php:434. The makeLink function expects a string a not an array. I fixed it on my mediawiki install (community.kde.org) by adding a if (is_array($attrs['class'])) { $attrs['class'] = implode( ' ', $attrs['class'] ); } but it's a dirty hack, the solution would be to imploding the array in the echo extension.

Expected Results:

No notice :)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Please provide a stack trace as well as versions of Echo and MW that you're using.

Aklapper changed the task status from Open to Stalled.Sep 18 2020, 3:13 PM

@Carl: After you have provided the information asked for, please set the status of this task back to "Open" via the Add Action...Change Status dropdown. Thanks!

Carl changed the task status from Stalled to Open.Sep 18 2020, 3:22 PM

Instance: https://community.kde.org/, https://techbase.kde.org/ and https://userbase.kde.org
Echo version: branch REL1_31 33e3eb299b28e784da465db3fc8ecd56fdf26e62
MW version: 1.34.2

 #	Time	Memory	Function	Location
1	0.0000	403736	{main}( )	.../index.php:0
2	0.0098	2590752	MediaWiki->run( )	.../index.php:44
3	0.0112	2698664	MediaWiki->main( )	.../MediaWiki.php:527
4	0.6281	6550128	MediaWiki->{closure:/srv/www/mediawiki/mediawiki-1.34.2/includes/MediaWiki.php:905-911}( )	.../MediaWiki.php:919
5	0.6281	6550128	OutputPage->output( )	.../MediaWiki.php:907
6	0.6292	6660760	SkinAether->outputPage( )	.../OutputPage.php:2574
7	0.6816	7432664	AetherTemplate->execute( )	.../SkinTemplate.php:217
8	0.6900	7450712	AetherTemplate->makeListItem( )	.../AetherTemplate.php:96
9	0.6900	7451088	AetherTemplate->makeLink( )	.../BaseTemplate.php:489
10	0.6912	7452464	implode ( )	.../BaseTemplate.php:435

Sorry I missed the first mail :(

In case this is needed and since the backtrace mention the theme installed, here is the source code for the theme: https://invent.kde.org/websites/aether-mediawiki

Echo version: branch REL1_31 33e3eb299b28e784da465db3fc8ecd56fdf26e62
MW version: 1.34.2

You are mixing very different branches... Could you try the REL1_34 branch of Echo if you run MediaWiki 1.34?

Oh sorry, it was a typo, I'm running the REL1_34 branch

@Mainframe98 Your recent commit a5a8f46958aa82adbb2bc8b531484025b145d5cf is related to this issue, but I’m not sure if you are aware of it given it is not mentionned in your commit.

I will test if it fixes this issue, but perhaps you already tested independently.

This sounds (and looks) suspiciously like the issue I encountered (and then fixed). If you could confirm that it fixes it, that would be great.

Note that the solution would be to imploding the array in the echo extension. would be inverse of what was recommended during code review of a5a8f46958aa82adbb2bc8b531484025b145d5cf, preferably, css classes are specified as an array to allow easier manipulation, and then have makeLink be responsible for doing the implode-ing.

@Mainframe98 thanks, unfortunately, I steal stuck in MediaWiki 1.34 due to lack of time to update, so I can't test it but it's reassuring that updating my MediaWiki installs should now correctly handle this case :)

Seb35 assigned this task to Mainframe98.

I was not able to reproduce it when I originally seen it with the skin Chameleon (versions changed since).

But I reproduced it with the skin Aether with MediaWiki 1.36.0-alpha with the recent patch and it is indeed fixed!

There is no issue with the skin Vector, I didn’t search why.

I did the maintenance changes below on the skin Aether (another small gift when you will update :-)

diff --git a/AetherTemplate.php b/AetherTemplate.php
index 72bf81c..181ba57 100644
--- a/AetherTemplate.php
+++ b/AetherTemplate.php
@@ -49 +49 @@ class AetherTemplate extends BaseTemplate {
-        $nav[$section][$key]['attributes'] = ' id="' . Sanitizer::escapeId( $xmlID ) . '"';
+        $nav[$section][$key]['attributes'] = ' id="' . Sanitizer::escapeIdForAttribute( $xmlID ) . '"';
@@ -365 +365 @@ class AetherTemplate extends BaseTemplate {
-      <div class="menu-title" id='<?php echo Sanitizer::escapeId( "p-$name" ) ?>' <?php echo Linker::tooltip( 'p-' . $name ) ?>>
+      <div class="menu-title" id='<?php echo Sanitizer::escapeIdForAttribute( "p-$name" ) ?>' <?php echo Linker::tooltip( 'p-' . $name ) ?>>
diff --git a/composer.json b/composer.json
index 87f4c46..eb8fcc4 100644
--- a/composer.json
+++ b/composer.json
@@ -3 +3,2 @@
-        "symfony/asset": "^4.3"
+        "symfony/asset": "^4.3",
+        "symfony/cache": "^5.2"