119e69b52f
Resolves: #2049194
335 lines
11 KiB
Diff
335 lines
11 KiB
Diff
From 49a950b9e0dc262fd20c28e21ee4815ea8efe758 Mon Sep 17 00:00:00 2001
|
|
From: Sebastian Keller <skeller@gnome.org>
|
|
Date: Tue, 16 Nov 2021 18:57:26 +0100
|
|
Subject: [PATCH 1/3] search: Split out the description highlighter into its
|
|
own class
|
|
|
|
No functional change yet, only preparation to allow adding a unit test
|
|
later on.
|
|
|
|
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2033>
|
|
---
|
|
js/misc/util.js | 38 +++++++++++++++++++++++++++++++++++++-
|
|
js/ui/search.js | 12 +++++-------
|
|
2 files changed, 42 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/js/misc/util.js b/js/misc/util.js
|
|
index 8139d3f47..d1a702960 100644
|
|
--- a/js/misc/util.js
|
|
+++ b/js/misc/util.js
|
|
@@ -1,7 +1,8 @@
|
|
// -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*-
|
|
/* exported findUrls, spawn, spawnCommandLine, spawnApp, trySpawnCommandLine,
|
|
formatTime, formatTimeSpan, createTimeLabel, insertSorted,
|
|
- ensureActorVisibleInScrollView, wiggle, lerp, GNOMEversionCompare */
|
|
+ ensureActorVisibleInScrollView, wiggle, lerp, GNOMEversionCompare,
|
|
+ Highlighter */
|
|
|
|
const { Clutter, Gio, GLib, Shell, St, GnomeDesktop } = imports.gi;
|
|
const Gettext = imports.gettext;
|
|
@@ -477,3 +478,38 @@ function GNOMEversionCompare(version1, version2) {
|
|
|
|
return 0;
|
|
}
|
|
+
|
|
+/* @class Highlighter Highlight given terms in text using markup. */
|
|
+var Highlighter = class {
|
|
+ /**
|
|
+ * @param {?string[]} terms - list of terms to highlight
|
|
+ */
|
|
+ constructor(terms) {
|
|
+ if (!terms)
|
|
+ return;
|
|
+
|
|
+ const escapedTerms = terms
|
|
+ .map(term => Shell.util_regex_escape(term))
|
|
+ .filter(term => term.length > 0);
|
|
+
|
|
+ if (escapedTerms.length === 0)
|
|
+ return;
|
|
+
|
|
+ this._highlightRegex = new RegExp('(%s)'.format(
|
|
+ escapedTerms.join('|')), 'gi');
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * Highlight all occurences of the terms defined for this
|
|
+ * highlighter in the provided text using markup.
|
|
+ *
|
|
+ * @param {string} text - text to highlight the defined terms in
|
|
+ * @returns {string}
|
|
+ */
|
|
+ highlight(text) {
|
|
+ if (!this._highlightRegex)
|
|
+ return text;
|
|
+
|
|
+ return text.replace(this._highlightRegex, '<b>$1</b>');
|
|
+ }
|
|
+};
|
|
diff --git a/js/ui/search.js b/js/ui/search.js
|
|
index 7300b053e..b1e76c46d 100644
|
|
--- a/js/ui/search.js
|
|
+++ b/js/ui/search.js
|
|
@@ -10,6 +10,8 @@ const ParentalControlsManager = imports.misc.parentalControlsManager;
|
|
const RemoteSearch = imports.ui.remoteSearch;
|
|
const Util = imports.misc.util;
|
|
|
|
+const { Highlighter } = imports.misc.util;
|
|
+
|
|
const SEARCH_PROVIDERS_SCHEMA = 'org.gnome.desktop.search-providers';
|
|
|
|
var MAX_LIST_SEARCH_RESULTS_ROWS = 5;
|
|
@@ -596,7 +598,7 @@ var SearchResultsView = GObject.registerClass({
|
|
|
|
this._providers = [];
|
|
|
|
- this._highlightRegex = null;
|
|
+ this._highlighter = new Highlighter();
|
|
|
|
this._searchSettings = new Gio.Settings({ schema_id: SEARCH_PROVIDERS_SCHEMA });
|
|
this._searchSettings.connect('changed::disabled', this._reloadRemoteProviders.bind(this));
|
|
@@ -739,8 +741,7 @@ var SearchResultsView = GObject.registerClass({
|
|
if (this._searchTimeoutId == 0)
|
|
this._searchTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 150, this._onSearchTimeout.bind(this));
|
|
|
|
- let escapedTerms = this._terms.map(term => Shell.util_regex_escape(term));
|
|
- this._highlightRegex = new RegExp('(%s)'.format(escapedTerms.join('|')), 'gi');
|
|
+ this._highlighter = new Highlighter(this._terms);
|
|
|
|
this.emit('terms-changed');
|
|
}
|
|
@@ -894,10 +895,7 @@ var SearchResultsView = GObject.registerClass({
|
|
if (!description)
|
|
return '';
|
|
|
|
- if (!this._highlightRegex)
|
|
- return description;
|
|
-
|
|
- return description.replace(this._highlightRegex, '<b>$1</b>');
|
|
+ return this._highlighter.highlight(description);
|
|
}
|
|
});
|
|
|
|
--
|
|
2.35.1
|
|
|
|
|
|
From 7c1abe1bd91ecf274d81e122035cbeeef6fd58d4 Mon Sep 17 00:00:00 2001
|
|
From: Sebastian Keller <skeller@gnome.org>
|
|
Date: Wed, 17 Nov 2021 02:50:39 +0100
|
|
Subject: [PATCH 2/3] util: Properly handle markup in highlighter
|
|
|
|
The code to highlight matches did not properly escape the passed in text
|
|
as for markup before adding its highlighting markup. This lead to some
|
|
search result descriptions not showing up, because their descriptions
|
|
contained characters, such as "<", that would have to be escaped when
|
|
used in markup or otherwise lead to invalid markup.
|
|
|
|
To work around this some search providers wrongly started escaping the
|
|
description on their end before sending them to gnome-shell. This lead
|
|
to another issue. Now if the highlighter was trying to highlight the
|
|
term "a", and the escaped description contained "'", the "a" in
|
|
that would be considered a match and surrounded by "<b></b>". This
|
|
however would also generate invalid markup, again leading to an error
|
|
and the description not being shown.
|
|
|
|
Fix this by always escaping the passed in string before applying the
|
|
highlights in such a way that there are no matches within entities.
|
|
|
|
This also means that search providers that escaped their description
|
|
strings will now show up with the markup syntax. This will have to be
|
|
fixed separately in the affected search providers.
|
|
|
|
Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4791
|
|
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2033>
|
|
---
|
|
js/misc/util.js | 21 +++++++++++++++++++--
|
|
1 file changed, 19 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/js/misc/util.js b/js/misc/util.js
|
|
index d1a702960..802398d18 100644
|
|
--- a/js/misc/util.js
|
|
+++ b/js/misc/util.js
|
|
@@ -508,8 +508,25 @@ var Highlighter = class {
|
|
*/
|
|
highlight(text) {
|
|
if (!this._highlightRegex)
|
|
- return text;
|
|
+ return GLib.markup_escape_text(text, -1);
|
|
+
|
|
+ let escaped = [];
|
|
+ let lastMatchEnd = 0;
|
|
+ let match;
|
|
+ while ((match = this._highlightRegex.exec(text))) {
|
|
+ if (match.index > lastMatchEnd) {
|
|
+ let unmatched = GLib.markup_escape_text(
|
|
+ text.slice(lastMatchEnd, match.index), -1);
|
|
+ escaped.push(unmatched);
|
|
+ }
|
|
+ let matched = GLib.markup_escape_text(match[0], -1);
|
|
+ escaped.push('<b>%s</b>'.format(matched));
|
|
+ lastMatchEnd = match.index + match[0].length;
|
|
+ }
|
|
+ let unmatched = GLib.markup_escape_text(
|
|
+ text.slice(lastMatchEnd), -1);
|
|
+ escaped.push(unmatched);
|
|
|
|
- return text.replace(this._highlightRegex, '<b>$1</b>');
|
|
+ return escaped.join('');
|
|
}
|
|
};
|
|
--
|
|
2.35.1
|
|
|
|
|
|
From 82e2a6dcfabc2f82efbf468175d16c303f0c73da Mon Sep 17 00:00:00 2001
|
|
From: Sebastian Keller <skeller@gnome.org>
|
|
Date: Wed, 17 Nov 2021 03:05:05 +0100
|
|
Subject: [PATCH 3/3] tests: Add unit test for highlighter
|
|
|
|
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2033>
|
|
---
|
|
tests/meson.build | 12 ++++-
|
|
tests/unit/highlighter.js | 106 ++++++++++++++++++++++++++++++++++++++
|
|
2 files changed, 117 insertions(+), 1 deletion(-)
|
|
create mode 100644 tests/unit/highlighter.js
|
|
|
|
diff --git a/tests/meson.build b/tests/meson.build
|
|
index c0431631f..50fb601e9 100644
|
|
--- a/tests/meson.build
|
|
+++ b/tests/meson.build
|
|
@@ -10,7 +10,17 @@ run_test = configure_file(
|
|
testenv = environment()
|
|
testenv.set('GSETTINGS_SCHEMA_DIR', join_paths(meson.build_root(), 'data'))
|
|
|
|
-foreach test : ['insertSorted', 'jsParse', 'markup', 'params', 'url', 'versionCompare']
|
|
+tests = [
|
|
+ 'highlighter',
|
|
+ 'insertSorted',
|
|
+ 'jsParse',
|
|
+ 'markup',
|
|
+ 'params',
|
|
+ 'url',
|
|
+ 'versionCompare',
|
|
+]
|
|
+
|
|
+foreach test : tests
|
|
test(test, run_test,
|
|
args: 'unit/@0@.js'.format(test),
|
|
env: testenv,
|
|
diff --git a/tests/unit/highlighter.js b/tests/unit/highlighter.js
|
|
new file mode 100644
|
|
index 000000000..d582d38e3
|
|
--- /dev/null
|
|
+++ b/tests/unit/highlighter.js
|
|
@@ -0,0 +1,106 @@
|
|
+// -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*-
|
|
+
|
|
+// Test cases for SearchResult description match highlighter
|
|
+
|
|
+const JsUnit = imports.jsUnit;
|
|
+const Pango = imports.gi.Pango;
|
|
+
|
|
+const Environment = imports.ui.environment;
|
|
+Environment.init();
|
|
+
|
|
+const Util = imports.misc.util;
|
|
+
|
|
+const tests = [
|
|
+ { input: 'abc cba',
|
|
+ terms: null,
|
|
+ output: 'abc cba' },
|
|
+ { input: 'abc cba',
|
|
+ terms: [],
|
|
+ output: 'abc cba' },
|
|
+ { input: 'abc cba',
|
|
+ terms: [''],
|
|
+ output: 'abc cba' },
|
|
+ { input: 'abc cba',
|
|
+ terms: ['a'],
|
|
+ output: '<b>a</b>bc cb<b>a</b>' },
|
|
+ { input: 'abc cba',
|
|
+ terms: ['a', 'a'],
|
|
+ output: '<b>a</b>bc cb<b>a</b>' },
|
|
+ { input: 'CaSe InSenSiTiVe',
|
|
+ terms: ['cas', 'sens'],
|
|
+ output: '<b>CaS</b>e In<b>SenS</b>iTiVe' },
|
|
+ { input: 'This contains the < character',
|
|
+ terms: null,
|
|
+ output: 'This contains the < character' },
|
|
+ { input: 'Don\'t',
|
|
+ terms: ['t'],
|
|
+ output: 'Don'<b>t</b>' },
|
|
+ { input: 'Don\'t',
|
|
+ terms: ['n\'t'],
|
|
+ output: 'Do<b>n't</b>' },
|
|
+ { input: 'Don\'t',
|
|
+ terms: ['o', 't'],
|
|
+ output: 'D<b>o</b>n'<b>t</b>' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['salt'],
|
|
+ output: '<b>salt</b>&pepper' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['salt', 'alt'],
|
|
+ output: '<b>salt</b>&pepper' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['pepper'],
|
|
+ output: 'salt&<b>pepper</b>' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['salt', 'pepper'],
|
|
+ output: '<b>salt</b>&<b>pepper</b>' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['t', 'p'],
|
|
+ output: 'sal<b>t</b>&<b>p</b>e<b>p</b><b>p</b>er' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['t', '&', 'p'],
|
|
+ output: 'sal<b>t</b><b>&</b><b>p</b>e<b>p</b><b>p</b>er' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['e'],
|
|
+ output: 'salt&p<b>e</b>pp<b>e</b>r' },
|
|
+ { input: 'salt&pepper',
|
|
+ terms: ['&a', '&am', '&', '&'],
|
|
+ output: 'salt&pepper' },
|
|
+ { input: '&&&&&',
|
|
+ terms: ['a'],
|
|
+ output: '&&&&&' },
|
|
+ { input: '&;&;&;&;&;',
|
|
+ terms: ['a'],
|
|
+ output: '&;&;&;&;&;' },
|
|
+ { input: '&;&;&;&;&;',
|
|
+ terms: [';'],
|
|
+ output: '&<b>;</b>&<b>;</b>&<b>;</b>&<b>;</b>&<b>;</b>' },
|
|
+ { input: '&',
|
|
+ terms: ['a'],
|
|
+ output: '&<b>a</b>mp;' }
|
|
+];
|
|
+
|
|
+try {
|
|
+ for (let i = 0; i < tests.length; i++) {
|
|
+ let highlighter = new Util.Highlighter(tests[i].terms);
|
|
+ let output = highlighter.highlight(tests[i].input);
|
|
+
|
|
+ JsUnit.assertEquals(`Test ${i + 1} highlight ` +
|
|
+ `"${tests[i].terms}" in "${tests[i].input}"`,
|
|
+ output, tests[i].output);
|
|
+
|
|
+ let parsed = false;
|
|
+ try {
|
|
+ Pango.parse_markup(output, -1, '');
|
|
+ parsed = true;
|
|
+ } catch (e) {}
|
|
+ JsUnit.assertEquals(`Test ${i + 1} is valid markup`, true, parsed);
|
|
+ }
|
|
+} catch (e) {
|
|
+ if (typeof(e.isJsUnitException) != 'undefined'
|
|
+ && e.isJsUnitException)
|
|
+ {
|
|
+ if (e.comment)
|
|
+ log(`Error in: ${e.comment}`);
|
|
+ }
|
|
+ throw e;
|
|
+}
|
|
--
|
|
2.35.1
|
|
|