Problem/Motivation

The Media module's settings UI contains a potential warning message that does not follow the standard t() function's :link syntax

Steps to reproduce

Review the buildForm() function in MediaSettingsForm.php.

Proposed resolution

Use :link in warning message

Remaining tasks

User interface changes

NA

API changes

NA

Data model changes

NA

Release notes snippet

NA

CommentFileSizeAuthor
#3 interdiff-3176229-2-3.txt1.15 KBmarkdorison
#3 3176229-3.patch1.13 KBmarkdorison
#2 3176229-2.patch1.11 KBchrisfree

Comments

chrisfree created an issue. See original summary.

chrisfree’s picture

chrisfree commented
Status: Needs work » Needs review
StatusFileSize
new 3176229-2.patch1.11 KB

Here is a patch to fix the preferred syntax issue.

markdorison’s picture

markdorison
he/him
commented
StatusFileSize
new 3176229-3.patch1.13 KB
new interdiff-3176229-2-3.txt1.15 KB

This change looks good to me. I added an additional wrapping of the replacements array to increase readability.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Postponed (maintainer needs more info)
Issue tags: -Quick fix, -translation +Bug Smash Initiative

Can you elaborate on why this is needed, if the link is a constant, is there a need to use :link?
I understand if the link were based on a variable, but here it's not.
In addition this will create a new string and break existing translations, so we need to have a reason to do it.

chrisfree’s picture

chrisfree commented

I must be misunderstanding when to use :link. I was working on #3173180: Add UI for 'loading' html attribute to images and was given the impression that using :link was the best practice. Now, after digging a bit, I see now that the filtering/escaping : will trigger is probably not necessary when the link is static. Is that correct?

I assume that means the other suggestion (comment #18 on #3173180: Add UI for 'loading' html attribute to images), is incorrect?

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Assigned: chrisfree » Unassigned
Status: Postponed (maintainer needs more info) » Closed (outdated)

I agree with #4 that in this case it doesn't matter but it would be good practice to use the :link (the docs say to use ':' for attributes).

But the line being changed in the patch was modified in #3142818: Remove target=_blank from links in media settings forms making this outdated.