From 74d917887869747fae564ac3915db21b14d186e6 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Tue, 26 Mar 2019 09:16:06 +0000 Subject: [PATCH] Bug 1538771 - Fix highlighting of matches within URLs in QuantumBar results. r=adw Differential Revision: https://phabricator.services.mozilla.com/D24762 --HG-- extra : moz-landing-system : lando --- browser/components/urlbar/UrlbarResult.jsm | 8 +- browser/components/urlbar/UrlbarView.jsm | 4 +- .../urlbar/tests/UrlbarTestUtils.jsm | 2 + .../urlbar/tests/browser/browser.ini | 1 + .../browser/browser_view_resultDisplay.js | 102 ++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 browser/components/urlbar/tests/browser/browser_view_resultDisplay.js diff --git a/browser/components/urlbar/UrlbarResult.jsm b/browser/components/urlbar/UrlbarResult.jsm index d3cb0d2f3605..69db09329a25 100644 --- a/browser/components/urlbar/UrlbarResult.jsm +++ b/browser/components/urlbar/UrlbarResult.jsm @@ -16,7 +16,9 @@ var EXPORTED_SYMBOLS = ["UrlbarResult"]; const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetters(this, { + BrowserUtils: "resource://gre/modules/BrowserUtils.jsm", Services: "resource://gre/modules/Services.jsm", + UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm", UrlbarUtils: "resource:///modules/UrlbarUtils.jsm", }); @@ -168,7 +170,11 @@ class UrlbarResult { if (payloadInfo.url) { // For display purposes we need to unescape the url. payloadInfo.displayUrl = [...payloadInfo.url]; - payloadInfo.displayUrl[0] = Services.textToSubURI.unEscapeURIForUI("UTF-8", payloadInfo.displayUrl[0]); + let url = payloadInfo.displayUrl[0]; + if (UrlbarPrefs.get("trimURLs")) { + url = BrowserUtils.trimURL(url || ""); + } + payloadInfo.displayUrl[0] = Services.textToSubURI.unEscapeURIForUI("UTF-8", url); } let entries = Object.entries(payloadInfo); diff --git a/browser/components/urlbar/UrlbarView.jsm b/browser/components/urlbar/UrlbarView.jsm index 47b0f9244d5f..cb7efd6f7be7 100644 --- a/browser/components/urlbar/UrlbarView.jsm +++ b/browser/components/urlbar/UrlbarView.jsm @@ -8,7 +8,6 @@ var EXPORTED_SYMBOLS = ["UrlbarView"]; const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetters(this, { - BrowserUtils: "resource://gre/modules/BrowserUtils.jsm", Services: "resource://gre/modules/Services.jsm", UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm", UrlbarTokenizer: "resource:///modules/UrlbarTokenizer.jsm", @@ -455,8 +454,7 @@ class UrlbarView { let setURL = () => { url = this._createElement("span"); url.className = "urlbarView-secondary urlbarView-url"; - let val = BrowserUtils.trimURL(result.payload.displayUrl || ""); - this._addTextContentWithHighlights(url, val, + this._addTextContentWithHighlights(url, result.payload.displayUrl, result.payloadHighlights.displayUrl || []); }; switch (result.type) { diff --git a/browser/components/urlbar/tests/UrlbarTestUtils.jsm b/browser/components/urlbar/tests/UrlbarTestUtils.jsm index 0d6734122bb2..5512033050a4 100644 --- a/browser/components/urlbar/tests/UrlbarTestUtils.jsm +++ b/browser/components/urlbar/tests/UrlbarTestUtils.jsm @@ -438,6 +438,7 @@ class UrlbarAbstraction { action: actionElement, row: element, separator: urlElement || actionElement, + title: element.getElementsByClassName("urlbarView-title")[0], url: urlElement, }; if (details.type == UrlbarUtils.RESULT_TYPE.SEARCH) { @@ -471,6 +472,7 @@ class UrlbarAbstraction { action: element._actionText, row: element, separator: element._separator, + title: element._titleText, url: element._urlText, }; if (details.type == UrlbarUtils.RESULT_TYPE.SEARCH && action) { diff --git a/browser/components/urlbar/tests/browser/browser.ini b/browser/components/urlbar/tests/browser/browser.ini index 4c5f4287e47e..0c9ca7641c75 100644 --- a/browser/components/urlbar/tests/browser/browser.ini +++ b/browser/components/urlbar/tests/browser/browser.ini @@ -152,3 +152,4 @@ support-files = [browser_urlbarValueOnTabSwitch.js] [browser_userTypedValue.js] support-files = file_userTypedValue.html +[browser_view_resultDisplay.js] diff --git a/browser/components/urlbar/tests/browser/browser_view_resultDisplay.js b/browser/components/urlbar/tests/browser/browser_view_resultDisplay.js new file mode 100644 index 000000000000..54feb17caae7 --- /dev/null +++ b/browser/components/urlbar/tests/browser/browser_view_resultDisplay.js @@ -0,0 +1,102 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +/** + * Tests that a result has the various elements displayed in the URL bar as + * we expect them to be. + */ + +add_task(async function setup() { + await PlacesUtils.history.clear(); + + registerCleanupFunction(async function() { + await PlacesUtils.history.clear(); + Services.prefs.clearUserPref("browser.urlbar.trimURLs"); + }); +}); + +async function testResult(input, expected) { + const ESCAPED_URL = encodeURI(input.url); + + await PlacesTestUtils.addVisits({ + uri: input.url, + title: input.title, + }); + + await promiseAutocompleteResultPopup("\u6e2C\u8a66"); + + let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 1); + Assert.equal(result.url, ESCAPED_URL, + "Should have the correct url to load"); + Assert.equal(result.displayed.url, expected.displayedUrl, + "Should have the correct displayed url"); + Assert.equal(result.displayed.title, input.title, + "Should have the expected title"); + Assert.equal(result.displayed.typeIcon, "none", + "Should not have a type icon"); + Assert.equal(result.image, `page-icon:${ESCAPED_URL}`, + "Should have the correct favicon"); + + assertDisplayedHighlights("title", result.element.title, expected.highlightedTitle); + + assertDisplayedHighlights("url", result.element.url, expected.highlightedUrl); +} + +function assertDisplayedHighlights(elementName, element, expectedResults) { + Assert.equal(element.childNodes.length, expectedResults.length, + `Should have the correct number of child nodes for ${elementName}`); + + for (let i = 0; i < element.childNodes.length; i++) { + let child = element.childNodes[i]; + Assert.equal(child.textContent, expectedResults[i][0], + `Should have the correct text for the ${i} part of the ${elementName}`); + Assert.equal(child.nodeName, expectedResults[i][1] ? "strong" : "#text", + `Should have the correct text/strong status for the ${i} part of the ${elementName}`); + } +} + +add_task(async function test_url_result() { + await testResult({ + query: "\u6e2C\u8a66", + title: "The \u6e2C\u8a66 URL", + url: "http://example.com/\u6e2C\u8a66test", + }, { + displayedUrl: "example.com/\u6e2C\u8a66test", + highlightedTitle: [ + ["The ", false], + ["\u6e2C\u8a66", true], + [" URL", false], + ], + highlightedUrl: [ + ["example.com/", false], + ["\u6e2C\u8a66", true], + ["test", false], + ], + }); +}); + +add_task(async function test_url_result_no_trimming() { + Services.prefs.setBoolPref("browser.urlbar.trimURLs", false); + + await testResult({ + query: "\u6e2C\u8a66", + title: "The \u6e2C\u8a66 URL", + url: "http://example.com/\u6e2C\u8a66test", + }, { + displayedUrl: "http://example.com/\u6e2C\u8a66test", + highlightedTitle: [ + ["The ", false], + ["\u6e2C\u8a66", true], + [" URL", false], + ], + highlightedUrl: [ + ["http://example.com/", false], + ["\u6e2C\u8a66", true], + ["test", false], + ], + }); + + Services.prefs.clearUserPref("browser.urlbar.trimURLs"); +});