Closed Tersaramor closed 6 months ago
MarkEWaite
commented
6 months ago Thanks for the details. I can duplicate the problem with a one line Pipeline script using the set of plugins and versions you provided:
addBadge(icon: '/images/48x48/light-grey.gif', text: 'large icon')The icon is 16x16 even though the source icon is using a 48x48 icon. This is the preferred result.
The icon is 48x48. This is not the preferred result.
MarkEWaite
commented
6 months ago @Tersaramor I've attempted a fix for the bug. Could you confirm that the incremental build works well in your environment?
timonwalkley
commented
6 months ago It looks like we're still experiencing this issue on v1.12.
The larger images are coming from /images/svg...
addBadge(icon: "/images/svgs/logo.svg")
strangelookingnerd
commented
6 months ago Hey @timonwalkley. Sorry you still need to deal with this issue. I can reproduce it, however I think it has to do with the SVG itself as it does have some sort of sizing attached to it which I'm not sure we can actually override. I'll look a little deeper into it and come back to you as soon as possible.
strangelookingnerd
commented
6 months ago From what I can tell this happens for the "core" icons that can be referenced as either addBadge(icon: "/images/svgs/logo.svg") or addBadge(icon: "icon-logo") (the latter not being supported yet by this plugin).
It appears that in icon.jelly it is determined hey, this is a core icon using Functions and IconSet. There it is decided hey, since this is a core icon it has this size which is then used for the style.
This functionallity is rather complex and can likely not be adapted even though one could argue that overridding the actual given css class could be considered a bug.
Maybe someone from @jenkinsci/sig-ux could share some insights here or confirm my observations.
As of now it seems the only remedy is to hard-code the icon style="width: 16px; height: 16px;" in the plugin.
timja
commented
6 months ago can't you add something like the class icon-sm ?
strangelookingnerd
commented
6 months ago We already do that, but it does not seem to work for these icons - or at least not always. On my refactoring branch (#151) I can see that:
<span class="${it.cssClass}" style="${it.style}">
<l:icon class="icon-sm" src="${it.icon}" alt="${it.text}" htmlTooltip="${it.text}"/>
</span>does produce icons with width and height set to 48px when used with addBadge(icon: "/images/svgs/logo.svg") whereas:
<span class="${it.cssClass}" style="${it.style}">
<l:icon class="icon-sm" style="width: 16px; height: 16px;" src="${it.icon}" alt="${it.text}" htmlTooltip="${it.text}"/>
</span>works perfectly fine. Right now I fail to reproduce it on master (but I'm certain I did just an hour ago)
timja
commented
6 months ago I would be wary of defaulting to html for tooltips but that code looks fine
strangelookingnerd
commented
6 months ago Ok, now I get it - there is something fishy with how the path to the icon is determined in my local environment. Anyways:
<l:icon src="/images/svgs/logo.svg" class="icon-sm" />results in the icon
However
<l:icon src="/jenkins/static/40c46706/images/svgs/logo.svg" class="icon-sm" />results in
timja
commented
6 months ago The icon code is a mess, you would need to experiment with it to understand why this is happening: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/layout/icon.jelly
I can't tell from just reading it.
strangelookingnerd
commented
6 months ago I tried to dig a little deeper into it and found that in IconSet#initializeSVGs all "core" svg are being added as icons for all the existing sizes. However in IconSet#addIcon they are added to the iconsByUrl map with their URL as key which is the same, no matter the size. Therefor the map contains those core icons only with the 48x48 / icon-xlg size. This map is used by icon.jelly via Functions#tryGetIcon
This means if i build an icon using /images/svgs/logo.svg I will always end up with the 48x48 variant. If i however use /jenkins/static/40c46706/images/svgs/logo.svg there is no entry for it in the map and I will therefor not use the icons style in icon.jelly
If I just use logo.svg I end up with the 24x24 variant because Functions#tryGetIcon will append the icon-md class.
I do agree this code is a mess and changing anything will end up causing more issues for sure. So the easiest remedy for me here is to hard-code the style or bypassing that issue with something like this.
timja
commented
6 months ago nice analysis. I'm sure it could be cleaned up, but would require some testing and analysis...
jonsgity
commented
6 months ago We are on Badge 1.12 and getting this sometimes when jobs are added to the queue:
Is this the same issue? It goes away after several seconds and after refreshing the page. After that, the job appear in the queue as usual.
MarkEWaite
commented
6 months ago @jonsgity that does not seem like it is related to the badge plugin or at least it is a very different behavior than any other report. Please open a new issue and include the stack trace that has likely been written to the Jenkins log file when that message appears.
You can also test the build of this pull request in your environment to confirm that it does not resolve the issue. The "Checks" tab includes a section called "Incrementals". The page in that section provides a URL where you can download the build of this pull request.
Jenkins and plugins versions report
Jenkins: 2.440.3, Java 11.0.13, and plugins
What Operating System are you using (both controller, and any agents involved in the problem)?
Ubuntu 20.04
Reproduction steps
addBadge(icon: relativeIconPath, text: displayName, link: url)Expected Results
Icon is 16x16
Actual Results
icon has actual size
Anything else?
Probably related to https://github.com/jenkinsci/badge-plugin/pull/130/files
Are you interested in contributing a fix?
No response