Problem/Motivation

Theme functions were removed in 8.x, but not correctly deprecated. In 9.x, they are deprecated, but with a nasty warning. Let's convert them to proper twig templates.

Steps to reproduce

Enable the module on Drupal 9, ensure warnings are visible on your site, view any page (that displays warnings) after a cache rebuild.

CommentFileSizeAuthor
#8 3183780-8.patch3.59 KBgnuget
#8 3183780-2-8-interdiff.txt751 bytesgnuget
#7 filefield_sources-twigify-3183780-7.patch4.11 KBrivimey
#5 filefield_sources-twigify-3183780-4.patch2.39 KBrivimey
#3 3183780-2-problems.png8.12 KBBohus Ulrych
#2 filefield_sources-twigify-3183780-2.patch3.77 KBdouggreen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

douggreen created an issue. See original summary.

douggreen’s picture

douggreen CreditAttribution: douggreen commented
Status: Active » Needs review
FileSize
filefield_sources-twigify-3183780-2.patch3.77 KB

Attached patch converts the theme functions to twig templates. I'm unfamiliar with this module, so this needs thorough review. I'm particularly concerned with using |raw in the element template, as this might have security implications.

Bohus Ulrych’s picture

Bohus Ulrych
Location Pilsen (Czechia)
CreditAttribution: Bohus Ulrych commented
FileSize
3183780-2-problems.png8.12 KB

Hi,
FYI this patch removed warnings, but also broke functionality.
No error on the page, in watchdog or console log.
Tested with D9.1.5 and module version 8.x-1.x-dev updated 15 Jan 2021 at 18:15 UTC
Thanks

Bohus Ulrych’s picture

Bohus Ulrych
Location Pilsen (Czechia)
CreditAttribution: Bohus Ulrych commented

Hello @douggreen
I believe I've found where is the issue.

In you patch you are replacing
- return '<div class="filefield-sources-list">' . implode(' | ', $links) . '</div>';
with
+<div class="fieldfield-sources-list">

You see? There is different class name filefield-sources-list VS fieldfield-sources-list
And this class name is then used in JavaScript file, with wrong class name it doesn't work.

After correcting this it works.

rivimey’s picture

rivimey
she/her
CreditAttribution: rivimey at IvimeyCom commented
FileSize
filefield_sources-twigify-3183780-4.patch2.39 KB

I've made a new patch, based on current HEAD, that implements the change in #4.

Status: Needs review » Needs work

The last submitted patch, 5: filefield_sources-twigify-3183780-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

rivimey’s picture

rivimey
she/her
CreditAttribution: rivimey at IvimeyCom commented
FileSize
filefield_sources-twigify-3183780-7.patch4.11 KB

Oops. sorry, let me format that again..

gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and commented
Status: Needs work » Needs review
FileSize
3183780-2-8-interdiff.txt751 bytes
3183780-8.patch3.59 KB

Here a new version of #2 with the changes suggested at #4.

This is looking good.

I will give it a few days and then I will commit it.

Thanks!!!

Bohus Ulrych’s picture

Bohus Ulrych
Location Pilsen (Czechia)
CreditAttribution: Bohus Ulrych commented

Tested (D9.1.7), works. Thanks

  • gnuget committed 84a6d46 on 8.x-1.x 
    Issue #3183780 by gnuget, douggreen, Bohus Ulrych: User deprecated...
gnuget’s picture

gnuget
Primary language Spanish
CreditAttribution: gnuget as a volunteer and commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

aiphes’s picture

aiphes
Primary language French
CreditAttribution: aiphes commented

I'm facing off this issue with File Field Sources (filefield_sources) 8.x-1.0-alpha4 on D9.2.7.
User deprecated function: Theme functions are deprecated in drupal:8.0.0 and are removed from drupal:10.0.0. Use Twig templates instead of theme_filefield_sources_list(). See https://www.drupal.org/node/1831138 in Drupal\Core\Theme\Registry->processExtension() (line 498 of core/lib/Drupal/Core/Theme/Registry.php).

which version do I use to avoid this warning ?

thanks