Problem/Motivation

https://www.drupal.org/project/drupal/issues/3535943

It was uncovered that the preprocess field precursors have been completed.

@mstrelan

I know we're just moving things around, but I don't think we should ignore this. The todo issue has been "fixed" for 10 years. So can we change this to use #[Hook('preprocess_field__comment')] or does that not work yet or will that never work?

@mstrelan

In fact I tested it by making the suggested change, putting a breakpoint in the method and running \Drupal\Tests\comment\Functional\CommentNonNodeTest.
Potentially we can remove the if ($element['#field_type'] == 'comment') condition as well.

@berdir

One problem is #1367354: The list of theme hook suggestions for "field" is incorrectly ordered and has namespacing collisions. This would also match all fields on a bundle named comment, as well as fields named comment, not just those of type comment

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3536196

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

nicxvan created an issue. See original summary.

mstrelan’s picture

mstrelan commented
Status: Active » Postponed

Postponed on #3535943: Convert final 4 preprocess hooks in core modules.

nicxvan’s picture

nicxvan commented
Title: Investigate preprocess_field in the comment module » [pp-1] Investigate preprocess_field in the comment module

Thanks!

mstrelan’s picture

mstrelan commented
Title: [pp-1] Investigate preprocess_field in the comment module » Investigate preprocess_field in the comment module
Status: Postponed » Active

Blocker is in

mstrelan opened merge request !12793

mstrelan’s picture

mstrelan commented
Status: Active » Needs review
dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

I don't know that there's much to say about this change. The hook is still invoked after applying the MR. Personally, I commented out the comment_form variable then rebuilt the cache and it stopped being rendered. The new comment is a good one to make sure no one tries to remove that condition later thinking that it's redundant. This looks good to me.

  • catch committed e6abe62c on 11.x 
    Issue #3536196 by mstrelan, nicxvan, dcam: Investigate preprocess_field...
catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#1367354: The list of theme hook suggestions for "field" is incorrectly ordered and has namespacing collisions

Yeah good to add the comment.

After committing I realised we have two issues open dealing with these collisions. We could have added a @todo, but seems silly to revert for that, so just adding as related issues.

Committed/pushed to 11.x, thanks!

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#3159050: Allow deprecating theme suggestions

catch closed merge request !12793

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

This might be the most comments trying to mark an issue fixed relative to the other comments on an issue.

Status: Fixed » Closed (fixed)

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