Skip to content

Commit ed14197

Browse files
Merge pull request #20099 from calixteman/bug1977259
[Editor] Fix the highlighting colors in HCM (bug 1977259)
2 parents f3080a1 + c022a32 commit ed14197

File tree

6 files changed

+98
-8
lines changed

6 files changed

+98
-8
lines changed

extensions/chromium/preferences_schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@
104104
},
105105
"highlightEditorColors": {
106106
"type": "string",
107-
"default": "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F"
107+
"default": "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F,yellow_HCM=#FFFFCC,green_HCM=#53FFBC,blue_HCM=#80EBFF,pink_HCM=#F6B8FF,red_HCM=#C50043"
108108
},
109109
"disableRange": {
110110
"title": "Disable range requests",

src/display/editor/highlight.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class HighlightEditor extends AnnotationEditor {
137137
return {
138138
action: "added",
139139
type: this.#isFreeHighlight ? "free_highlight" : "highlight",
140-
color: this._uiManager.highlightColorNames.get(this.color),
140+
color: this._uiManager.getNonHCMColorName(this.color),
141141
thickness: this.#thickness,
142142
methodOfCreation: this.#methodOfCreation,
143143
};
@@ -147,7 +147,7 @@ class HighlightEditor extends AnnotationEditor {
147147
get telemetryFinalData() {
148148
return {
149149
type: "highlight",
150-
color: this._uiManager.highlightColorNames.get(this.color),
150+
color: this._uiManager.getNonHCMColorName(this.color),
151151
};
152152
}
153153

@@ -374,7 +374,7 @@ class HighlightEditor extends AnnotationEditor {
374374
this._reportTelemetry(
375375
{
376376
action: "color_changed",
377-
color: this._uiManager.highlightColorNames.get(color),
377+
color: this._uiManager.getNonHCMColorName(color),
378378
},
379379
/* mustWait = */ true
380380
);
@@ -1024,7 +1024,9 @@ class HighlightEditor extends AnnotationEditor {
10241024
}
10251025

10261026
const rect = this.getRect(0, 0);
1027-
const color = AnnotationEditor._colorManager.convert(this.color);
1027+
const color = AnnotationEditor._colorManager.convert(
1028+
this._uiManager.getNonHCMColor(this.color)
1029+
);
10281030

10291031
const serialized = {
10301032
annotationType: AnnotationEditorType.HIGHLIGHT,

src/display/editor/tools.js

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -960,10 +960,10 @@ class AnnotationEditorUIManager {
960960
);
961961
}
962962

963-
get highlightColors() {
963+
get _highlightColors() {
964964
return shadow(
965965
this,
966-
"highlightColors",
966+
"_highlightColors",
967967
this.#highlightColors
968968
? new Map(
969969
this.#highlightColors.split(",").map(pair => {
@@ -976,6 +976,26 @@ class AnnotationEditorUIManager {
976976
);
977977
}
978978

979+
get highlightColors() {
980+
const { _highlightColors } = this;
981+
if (!_highlightColors) {
982+
return shadow(this, "highlightColors", null);
983+
}
984+
const map = new Map();
985+
const hasHCM = !!this.#pageColors;
986+
for (const [name, color] of _highlightColors) {
987+
const isNameForHCM = name.endsWith("_HCM");
988+
if (hasHCM && isNameForHCM) {
989+
map.set(name.replace("_HCM", ""), color);
990+
continue;
991+
}
992+
if (!hasHCM && !isNameForHCM) {
993+
map.set(name, color);
994+
}
995+
}
996+
return shadow(this, "highlightColors", map);
997+
}
998+
979999
get highlightColorNames() {
9801000
return shadow(
9811001
this,
@@ -986,6 +1006,18 @@ class AnnotationEditorUIManager {
9861006
);
9871007
}
9881008

1009+
getNonHCMColor(color) {
1010+
if (!this._highlightColors) {
1011+
return color;
1012+
}
1013+
const colorName = this.highlightColorNames.get(color);
1014+
return this._highlightColors.get(colorName) || color;
1015+
}
1016+
1017+
getNonHCMColorName(color) {
1018+
return this.highlightColorNames.get(color) || color;
1019+
}
1020+
9891021
/**
9901022
* Set the current drawing session.
9911023
* @param {AnnotationEditorLayer} layer

test/integration/highlight_editor_spec.mjs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,4 +2750,55 @@ describe("Highlight Editor", () => {
27502750
);
27512751
});
27522752
});
2753+
2754+
describe("Highlight color in HCM", () => {
2755+
let pages;
2756+
2757+
beforeEach(async () => {
2758+
pages = await loadAndWait(
2759+
"tracemonkey.pdf",
2760+
".annotationEditorLayer",
2761+
null,
2762+
null,
2763+
{
2764+
highlightEditorColors: "red=#AB0000,red_HCM=#00AB00",
2765+
forcePageColors: true,
2766+
pageColorsForeground: "#74ffd0",
2767+
pageColorsBackground: "#392a4f",
2768+
}
2769+
);
2770+
});
2771+
2772+
afterEach(async () => {
2773+
await closePages(pages);
2774+
});
2775+
2776+
it("must highlight with red color", async () => {
2777+
await Promise.all(
2778+
pages.map(async ([browserName, page]) => {
2779+
await switchToHighlight(page);
2780+
2781+
const rect = await getSpanRectFromText(page, 1, "Abstract");
2782+
const x = rect.x + rect.width / 2;
2783+
const y = rect.y + rect.height / 2;
2784+
await page.mouse.click(x, y, { count: 2, delay: 100 });
2785+
2786+
const editorSelector = getEditorSelector(0);
2787+
await page.waitForSelector(editorSelector);
2788+
await page.waitForSelector(
2789+
`.page[data-page-number = "1"] svg.highlightOutline.selected`
2790+
);
2791+
2792+
await page.waitForSelector(
2793+
`.page[data-page-number = "1"] .canvasWrapper > svg.highlight[fill = "#00AB00"]`
2794+
);
2795+
await waitForSerialized(page, 1);
2796+
const serialized = await getSerialized(page);
2797+
expect(serialized[0].color)
2798+
.withContext(`In ${browserName}`)
2799+
.toEqual([0xab, 0x00, 0x00]);
2800+
})
2801+
);
2802+
});
2803+
});
27532804
});

web/app.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,9 @@ const PDFViewerApplication = {
375375
spreadModeOnLoad: x => parseInt(x),
376376
supportsCaretBrowsingMode: x => x === "true",
377377
viewerCssTheme: x => parseInt(x),
378+
forcePageColors: x => x === "true",
379+
pageColorsBackground: x => x,
380+
pageColorsForeground: x => x,
378381
});
379382
}
380383

web/app_options.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,9 @@ const defaultOptions = {
286286
},
287287
highlightEditorColors: {
288288
/** @type {string} */
289-
value: "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F",
289+
value:
290+
"yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F," +
291+
"yellow_HCM=#FFFFCC,green_HCM=#53FFBC,blue_HCM=#80EBFF,pink_HCM=#F6B8FF,red_HCM=#C50043",
290292
kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
291293
},
292294
historyUpdateUrl: {

0 commit comments

Comments
 (0)