Follow-up to #2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests

Problem/Motivation

In order to make #2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests approachable, we need to break it up into smaller chunks. This issue address !placeholder in the views module

File Bounds:

core/modules/views/*

See #2506427: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand for complete motivation on removal of !placeholder

Proposed resolution

Replace !placeholder with @placeholder in the contact module.

Remaining tasks

  1. Replace !placeholder with @placeholder. Refer to patch in #2506445-85: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests as that patch should have related update
  2. Ensure tests come back clean
  3. Manually test the update and post screen shot after patch, review source for any difference in escaping.

User interface changes

CommentFileSizeAuthor
#13 replace_placeholder-views-2559441-13.patch22.75 KBsutharsan
#9 replace_placeholder-views-2559441-9.patch22.67 KBgeertvd
#8 replace_placeholder-2559441-8.patch26.45 KBjoelpittet
#8 interdiff.txt765 bytesjoelpittet
#5 replace_placeholder_views-2559441-5.patch25.84 KBgeertvd
#4 replace_placeholder_views-2559441-4.patch511.89 KBgeertvd

Comments

justAChris created an issue. See original summary.

justachris’s picture

justachris commented
Issue summary: View changes
Parent issue: #2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests » #2506427: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand
Related issues: +#2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests

Update IS for file bounds

justachris’s picture

justachris commented
Component: base system » views.module
Issue summary: View changes
geertvd’s picture

geertvd commented
Status: Active » Needs review
StatusFileSize
new replace_placeholder_views-2559441-4.patch511.89 KB
geertvd’s picture

geertvd commented
StatusFileSize
new replace_placeholder_views-2559441-5.patch25.84 KB

Last patch had a bit to much

geertvd’s picture

geertvd commented

Status: Needs review » Needs work

The last submitted patch, 5: replace_placeholder_views-2559441-5.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt765 bytes
new replace_placeholder-2559441-8.patch26.45 KB

Thanks @geertvd there is just one view being tested in the core/modules/block/src/Tests/BlockXssTest.php

(which will likely break the block tests, so I'll remove it from that patch)

geertvd’s picture

geertvd commented
StatusFileSize
new replace_placeholder-views-2559441-9.patch22.67 KB

Removing replacements in hook_help since this is covered in #2560783: Replace !placeholder with :placeholder for URLs in hook_help() implementations

justachris’s picture

justachris commented
Status: Needs review » Postponed

Postponed on determining plan in parent #2506427: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand and then analyzing whether this individual issue still makes sense.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented 
+++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
@@ -95,7 +95,7 @@ function operators() {
-        'short' => $this->t('!has'),
+        'short' => $this->t('@has'),

I think this is a false positive. (aka mistake change)

Some of this is being fixed by this #2567159: Fix improper usage of t() in ViewsBlock

justachris’s picture

justachris commented
Status: Postponed » Closed (duplicate)

Closing this, splitting by module was not the ideal approach to removing !placeholder. Marking as duplicate of #2506427: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand, since the chosen approach is / will be outlined there, please refer to it for any additional action.

sutharsan’s picture

sutharsan commented
Status: Closed (duplicate) » Needs review
StatusFileSize
new replace_placeholder-views-2559441-13.patch22.75 KB

Rerolling patch for easy migration into single patch at #2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests.
Changing status for test bot. Do revert status after test.

sutharsan’s picture

sutharsan commented
Status: Needs review » Closed (duplicate)

Status back to 'Closed (duplicate)'. Patch now included in #2506445: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests.