Skip to content

Commit f3bb56e

Browse files
brechtvlbrechtvl
committed
Address review comments including XSS, but no change to alt key
1 parent cc74015 commit f3bb56e

File tree

11 files changed

+35
-38
lines changed

11 files changed

+35
-38
lines changed

‎models/issues/issue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ func (ts labelSorter) Swap(i, j int) {
541541
// Ensure only one label of a given scope exists, with labels at the end of the
542542
// array getting preference over earlier ones.
543543
func RemoveDuplicateExclusiveLabels(labels []*Label) []*Label {
544-
var validLabels []*Label
544+
validLabels := make([]*Label, 0, len(labels))
545545

546546
for i, label := range labels {
547547
scope := label.ExclusiveScope()

‎models/issues/label.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,21 +175,21 @@ func (label *Label) ColorRGB() (float64, float64, float64, error) {
175175
return r, g, b, nil
176176
}
177177

178-
// Determine if label text should be light or dark to be readable on
179-
// background color.
178+
// Determine if label text should be light or dark to be readable on background color
180179
func (label *Label) UseLightTextColor() bool {
181180
if strings.HasPrefix(label.Color, "#") {
182181
if r, g, b, err := label.ColorRGB(); err == nil {
183-
// sRGB color space luminance
184-
luminance := (0.299*r + 0.587*g + 0.114*b) / 255
185-
return luminance < 0.35
182+
// Perceived brightness from: https://www.w3.org/TR/AERT/#color-contrast
183+
// In the future WCAG 3 APCA may be a better solution
184+
brightness := (0.299*r + 0.587*g + 0.114*b) / 255
185+
return brightness < 0.35
186186
}
187187
}
188188

189189
return false
190190
}
191191

192-
// Return scope substring of label name, or empty string if none exists.
192+
// Return scope substring of label name, or empty string if none exists
193193
func (label *Label) ExclusiveScope() string {
194194
if !label.Exclusive {
195195
return ""

‎models/migrations/v1_19/v244.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
package v1_19 //nolint
55

66
import (
7-
"fmt"
8-
97
"xorm.io/xorm"
108
)
119

@@ -14,8 +12,5 @@ func AddExclusiveLabel(x *xorm.Engine) error {
1412
Exclusive bool
1513
}
1614

17-
if err := x.Sync2(new(Label)); err != nil {
18-
return fmt.Errorf("Sync2: %w", err)
19-
}
20-
return nil
15+
return x.Sync(new(Label))
2116
}

‎modules/templates/helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ func RenderLabel(label *issues_model.Label) string {
815815
textColor = "#eee"
816816
}
817817

818-
description := emoji.ReplaceAliases(label.Description)
818+
description := emoji.ReplaceAliases(template.HTMLEscapeString(label.Description))
819819

820820
if labelScope == "" {
821821
// Regular label

‎routers/web/repo/issue.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
332332
labels = append(labels, orgLabels...)
333333
}
334334

335-
var labelExclusiveScopes []string
335+
// Get the exclusive scope for every label ID
336+
labelExclusiveScopes := make([]string, 0, len(labelIDs))
336337
for _, labelID := range labelIDs {
337338
foundExclusiveScope := false
338339
for _, label := range labels {

‎templates/repo/issue/labels/label_list.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@
4444
<div class="three wide column">
4545
{{if and (not $.PageIsOrgSettingsLabels ) (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}}
4646
<a class="ui right delete-button" href="#" data-url="{{$.Link}}/delete" data-id="{{.ID}}">{{svg "octicon-trash"}} {{$.locale.Tr "repo.issues.label_delete"}}</a>
47-
<a class="ui right edit-label-button" href="#" data-id="{{.ID}}" data-title="{{.Name}}" data-exclusive="{{.Exclusive}}" data-description="{{.Description}}" data-color={{.Color}}>{{svg "octicon-pencil"}} {{$.locale.Tr "repo.issues.label_edit"}}</a>
47+
<a class="ui right edit-label-button" href="#" data-id="{{.ID}}" data-title="{{.Name}}" {{if .Exclusive}}data-exclusive{{end}} data-description="{{.Description}}" data-color={{.Color}}>{{svg "octicon-pencil"}} {{$.locale.Tr "repo.issues.label_edit"}}</a>
4848
{{else if $.PageIsOrgSettingsLabels}}
4949
<a class="ui right delete-button" href="#" data-url="{{$.Link}}/delete" data-id="{{.ID}}">{{svg "octicon-trash"}} {{$.locale.Tr "repo.issues.label_delete"}}</a>
50-
<a class="ui right edit-label-button" href="#" data-id="{{.ID}}" data-title="{{.Name}}" data-exclusive="{{.Exclusive}}" data-description="{{.Description}}" data-color={{.Color}}>{{svg "octicon-pencil"}} {{$.locale.Tr "repo.issues.label_edit"}}</a>
50+
<a class="ui right edit-label-button" href="#" data-id="{{.ID}}" data-title="{{.Name}}" {{if .Exclusive}}data-exclusive{{end}} data-description="{{.Description}}" data-color={{.Color}}>{{svg "octicon-pencil"}} {{$.locale.Tr "repo.issues.label_edit"}}</a>
5151
{{end}}
5252
</div>
5353
</div>

‎web_src/js/components/ContextPopup.vue

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,10 @@
2626
<script>
2727
import $ from 'jquery';
2828
import {SvgIcon} from '../svg.js';
29+
import {useLightTextOnBackground} from '../utils.js';
2930
3031
const {appSubUrl, i18n} = window.config;
3132
32-
// NOTE: see models/issue_label.go for similar implementation
33-
const useLightTextColor = (label) => {
34-
// sRGB color space luminance
35-
const r = parseInt(label.color.substring(0, 2), 16);
36-
const g = parseInt(label.color.substring(2, 4), 16);
37-
const b = parseInt(label.color.substring(4, 6), 16);
38-
const luminance = (0.299 * r + 0.587 * g + 0.114 * b) / 255;
39-
return luminance < 0.35;
40-
};
41-
4233
export default {
4334
components: {SvgIcon},
4435
data: () => ({
@@ -86,7 +77,7 @@ export default {
8677
labels() {
8778
return this.issue.labels.map((label) => {
8879
let textColor;
89-
if (useLightTextColor(label)) {
80+
if (useLightTextOnBackground(label.color)) {
9081
textColor = '#eeeeee';
9182
} else {
9283
textColor = '#111111';

‎web_src/js/features/comp/LabelEdit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function initCompLabelEdit(selector) {
2020
$('.edit-label .color-picker').minicolors('value', $(this).data('color'));
2121
$('#label-modal-id').val($(this).data('id'));
2222
$('.edit-label .new-label-input').val($(this).data('title'));
23-
$('.edit-label .new-label-exclusive').prop('checked', $(this).data('exclusive'));
23+
$('.edit-label .new-label-exclusive').prop('checked', this.hasAttribute('data-exclusive'));
2424
$('.edit-label .new-label-desc-input').val($(this).data('description'));
2525
$('.edit-label .color-picker').val($(this).data('color'));
2626
$('.edit-label .minicolors-swatch-color').css('background-color', $(this).data('color'));

‎web_src/js/features/repo-legacy.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export function initRepoCommentForm() {
117117

118118
$(this).parent().find('.item').each(function () {
119119
if (scope) {
120-
/* Enable only clicked item for scoped labels. */
120+
// Enable only clicked item for scoped labels
121121
if ($(this).attr('data-scope') !== scope) {
122122
return true;
123123
}
@@ -129,7 +129,7 @@ export function initRepoCommentForm() {
129129
return true;
130130
}
131131
} else if (!$(this).is(clickedItem)) {
132-
/* Toggle for other labels. */
132+
// Toggle for other labels
133133
return true;
134134
}
135135

‎web_src/js/features/repo-projects.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import $ from 'jquery';
2+
import {useLightTextOnBackground} from '../utils.js';
23

34
const {csrfToken} = window.config;
45

@@ -183,13 +184,7 @@ export function initRepoProject() {
183184
}
184185

185186
function setLabelColor(label, color) {
186-
// sRGB color space luminance
187-
const r = parseInt(color.slice(1, 3), 16);
188-
const g = parseInt(color.slice(3, 5), 16);
189-
const b = parseInt(color.slice(5, 7), 16);
190-
const luminance = (0.299 * r + 0.587 * g + 0.114 * b) / 255;
191-
192-
if (luminance < 0.35) {
187+
if (useLightTextOnBackground(color)) {
193188
label.removeClass('dark-label').addClass('light-label');
194189
} else {
195190
label.removeClass('light-label').addClass('dark-label');

‎web_src/js/utils.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,18 @@ export function toAbsoluteUrl(url) {
146146
}
147147
return `${window.location.origin}${url}`;
148148
}
149+
150+
// determine if light or dark text color should be used on a given background color
151+
// NOTE: see models/issue_label.go for similar implementation
152+
export function useLightTextOnBackground(backgroundColor) {
153+
if (backgroundColor[0] === '#') {
154+
backgroundColor = backgroundColor.substring(1);
155+
}
156+
// Perceived brightness from: https://www.w3.org/TR/AERT/#color-contrast
157+
// In the future WCAG 3 APCA may be a better solution.
158+
const r = parseInt(backgroundColor.substring(0, 2), 16);
159+
const g = parseInt(backgroundColor.substring(2, 4), 16);
160+
const b = parseInt(backgroundColor.substring(4, 6), 16);
161+
const brightness = (0.299 * r + 0.587 * g + 0.114 * b) / 255;
162+
return brightness < 0.35;
163+
}

0 commit comments

Comments
 (0)