Skip to content

Fix for #56295 - Some Avatars In Commit Trailers Not Visible

What does this MR do?

This is a fix for issue #56295 (closed).

User avatars hosted by Gitlab (as opposed to Gravatar-hosted ones) are not visible in commit trailers.

This MR resolves this issue and all avatars are now visible in commit trailers.

Screenshots

Before After
avatar_before avatar_after

Details

Avatar images in commit trailers require special treatment because they are processed the same way as markdown.

Because markdown is allowed to reference files found only within a repo, any path-only link found in markdown will be automatically prefixed with the path of its repo, see: L132 in relative_link_filter.rb

As an example:

/uploads/-/system/user/avatar/51/avatar.png?width=16

becomes

/my_username/my_repo/uploads/-/system/user/avatar/51/avatar.png?width=16

Any avatar hosted by Gitlab (as opposed to ones hosted by Gravatar) will have a path-only link associated with it and, if found in markdown, will automatically received this prefix. When that happens, the url no longer points to to anything and the image for the avatar never appears.

To resolve this, I included an "only_path: false" option when retrieving the avatar's link, see: L92 in commit_trailers_filter.rb

After that, I then needed to add a small change within "avatar_helper.rb" to ensure that the option would "bubble up" through the chain of function calls, see: L60 - L66 in avatar_helper.rb

Lastly, I noticed that for the case when a commit trailer is written with an email unregistered with any Gitlab account, the alt text for the avatar renders as 's avatar. I added logic so it will now render as default avatar, see: L79 in avatar_helper.rb

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

What risks does this change pose? How might it affect the quality/performance of the product?

This MR changes the logic in building the path for an avatar within a commit trailer. Risks would include any unforeseen errors in building the path and breaking links to the avatars.

What additional test coverage or changes to tests will be needed?

A test for the new behavior of the markdown filtering (adding full paths to the avatars) was added, see: markdown_helper_spec.rb

Tests were added to ensure the "bubble up" behavior regarding the "only_path" option, see: L328 - L360 in avatar_helper_spec.rb

One test was added to ensure that commit filters with unregistered emails still receive alt text for the generic avatars, see: L362 in avatar_helper_spec.rb

Will it require cross-browser testing?

No

Closes #56295 (closed)

Edited by Jesse Hall

Merge request reports