Issue #3257729: Formatters shouldn't repeat core code
Merge request reports
Activity
added 5 commits
-
dcea1889...6527909c - 2 commits from branch
project:3.x - 2ac98bb7 - 3257729: Formatters should not repeat core code.
- 8ed23e67 - Fix cache context handling
- b445d9d1 - Merge branch '3257729-formatters-shouldnt-repeat' of...
Toggle commit list-
dcea1889...6527909c - 2 commits from branch
Resolved by Joshua SedlerCould it get rebased please?
- Last reply by Christopher Lewis
added 5 commits
- 16fb5454 - Upload New File
- 70e6fae8 - Issue #3451339: Automated Drupal 11 compatibility fixes for svg_image
- 40436d74 - Issue #3475964 by mably: Drupal 11 compatibility fixes for svg_image
- e210eeb6 - Issue #3420037: SvgImageWidget should inherit from ImageWidget instead of FileWidget
- de51c4b6 - Use existing fileUrlGenerator variable
Toggle commit listadded 8 commits
-
de51c4b6...535a6235 - 5 commits from branch
project:3.x - 416b758e - 3257729: Formatters should not repeat core code.
- fd0295c5 - Fix cache context handling
- 81b9913c - Use existing fileUrlGenerator variable
Toggle commit list-
de51c4b6...535a6235 - 5 commits from branch
added 8 commits
Toggle commit listadded 1 commit
- e9278f85 - Bring back the $imageStyle variable that is now used for dimension transformation. Fix phpcs.
added 7 commits
-
0951b328 - 1 commit from branch
project:3.x - 6a6a7e75 - 3257729: Formatters should not repeat core code.
- 9546e41a - Fix cache context handling
- 75ee12df - Use existing fileUrlGenerator variable
- 57d4ae80 - Tidy up use statements
- e3a12072 - Fix another phpcs issue
- 7d9db7d3 - Bring back the $imageStyle variable that is now used for dimension transformation. Fix phpcs.
Toggle commit list-
0951b328 - 1 commit from branch
added 1 commit
- a8a6f062 - Simplify code further, bring back early opt-out
94 foreach ($imageStyles as $image_style) { 95 $cacheTags = Cache::mergeTags($cacheTags, $image_style->getCacheTags()); 96 } 97 78 98 79 $svgAttributes = $this->getSetting('svg_attributes'); 80 99 81 foreach ($files as $delta => $file) { 100 $attributes = []; 82 $element = &$elements[$delta]; 101 83 $isSvg = svg_image_is_file_svg($file); 102 84 103 if ($isSvg) { 85 $cacheTags = Cache::mergeTags($element['#cache']['tags'], $file->getCacheTags()); 86 87 if (!$isSvg) { 88 $elements[$delta]['#cache']['tags'] = $cacheTags; -
Exactly, may be we can remove it for better clarity.
@Grevil what do you think?
EDIT: it looks like it was added in the original commit (issue #2981263).
Edited by Frank Mably changed this line in version 13 of the diff
-
Are you sure it was completely unneeded?
I was saying that we could remove
$cacheTagsfrom the!$isSvgpart of the condition, not the other part...Edited by Frank Mably -
The fact is that in the
$isSvgpart, the responsive image formatter does exactly the same as the non-responsive image formatter: return animage_formatterrender array without image style (when "render as image" is active) or the raw SVG contents. In both cases, the SvgImageFormatter does not add additional cache tags.That's why I didn't see a reason to keep the cache tags in the SvgResponsiveImageFormatter. That way, we are further unifying the two formatters (see also #3477774).
-
Ok for me then, @Grevil you can merge.
-
Had another look at this and was able to simplify the code further by removing more code that's already contained in the parent formatters. Also, I removed some cache handling code that had already been removed in HEAD but was brought back to this branch by mistake.
Also added a few comments as requested by @Grevil.
Last but not least, I noticed that there is a bug in SvgResponsiveImageFormatter not linking the image when we're dealing with an inline SVG. But that's out of scope of this issue; will open a separate one.
added 8 commits
-
10c0c556 - 1 commit from branch
project:3.x - 6e10b2f8 - 3257729: Formatters should not repeat core code.
- 98eb89c9 - Fix cache context handling
- dc41559a - Use existing fileUrlGenerator variable
- 42cbeee9 - Tidy up use statements
- 77d1ee7a - Fix another phpcs issue
- dcd8052c - Bring back the $imageStyle variable that is now used for dimension transformation. Fix phpcs.
- bc330b82 - Simplify code further, bring back early opt-out
Toggle commit list-
10c0c556 - 1 commit from branch
added 1 commit
