path: root/fix-markup-in-highlighter.patch

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 "&apos;", 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 &lt; character' },
+    { input: 'Don\'t',
+      terms: ['t'],
+      output: 'Don&apos;<b>t</b>' },
+    { input: 'Don\'t',
+      terms: ['n\'t'],
+      output: 'Do<b>n&apos;t</b>' },
+    { input: 'Don\'t',
+      terms: ['o', 't'],
+      output: 'D<b>o</b>n&apos;<b>t</b>' },
+    { input: 'salt&pepper',
+      terms: ['salt'],
+      output: '<b>salt</b>&amp;pepper' },
+    { input: 'salt&pepper',
+      terms: ['salt', 'alt'],
+      output: '<b>salt</b>&amp;pepper' },
+    { input: 'salt&pepper',
+      terms: ['pepper'],
+      output: 'salt&amp;<b>pepper</b>' },
+    { input: 'salt&pepper',
+      terms: ['salt', 'pepper'],
+      output: '<b>salt</b>&amp;<b>pepper</b>' },
+    { input: 'salt&pepper',
+      terms: ['t', 'p'],
+      output: 'sal<b>t</b>&amp;<b>p</b>e<b>p</b><b>p</b>er' },
+    { input: 'salt&pepper',
+      terms: ['t', '&', 'p'],
+      output: 'sal<b>t</b><b>&amp;</b><b>p</b>e<b>p</b><b>p</b>er' },
+    { input: 'salt&pepper',
+      terms: ['e'],
+      output: 'salt&amp;p<b>e</b>pp<b>e</b>r' },
+    { input: 'salt&pepper',
+      terms: ['&a', '&am', '&amp', '&amp;'],
+      output: 'salt&amp;pepper' },
+    { input: '&&&&&',
+      terms: ['a'],
+      output: '&amp;&amp;&amp;&amp;&amp;' },
+    { input: '&;&;&;&;&;',
+      terms: ['a'],
+      output: '&amp;;&amp;;&amp;;&amp;;&amp;;' },
+    { input: '&;&;&;&;&;',
+      terms: [';'],
+      output: '&amp;<b>;</b>&amp;<b>;</b>&amp;<b>;</b>&amp;<b>;</b>&amp;<b>;</b>' },
+    { input: '&amp;',
+      terms: ['a'],
+      output: '&amp;<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