From 894f274b9517123b365aecff61c37783e54aa5ff Mon Sep 17 00:00:00 2001 From: Drew Willcoxon Date: Thu, 6 Sep 2018 00:20:45 +0000 Subject: [PATCH] Bug 1484737 - Improve the handling of search alias highlighting in the urlbar. r=Mardak,mak This has two parts: (1) urlbar already had a formatValue method. Right now, it only does the URL formatting (domain highlighting, crossing out https for mixed content pages) that we do when the urlbar is not focused. This patch generalizes that method into a kind of "any formatting you want to do, do it here" method, and it adds alias formatting. formatValue is called by the base autocomplete binding when `value` is set. So it's called when the selection in the popup changes and the autocomplete controller subsequently sets the input value. (It's also called by urlbar on focus and blur.) And if anyone else sets the value directly, it'll be called then too of course. But it's not called when you're just typing in the input, so I added a call in urlbar.onResultsAdded, where we were calling highlightSearchAlias, to handle the first heuristic result being added or modified as a result of what you type. So I think that should cover all possible times we need to highlight the alias? (2) Just looking at the selected result to get the alias in the input doesn't work all the time. If you click a search tile on newtab and then key around in the popup, sometimes when you key down to the one-off buttons, the input value reverts to the alias (it's the user-typed value I guess?), but at the time that the value setter is called during the revert, the popup's selected index is still the last selection in the popup. IOW the selected index doesn't match up with what's in the input. Rather than deal with that, it seems safer to call PlacesSearchAutocompleteProvider.findMatchByAlias() on the first word in the input. But that has a couple of problems. It's async, and I noticed there can be a slight delay in the highlighting appearing. Also, we've already gotten the information returned by that method, when we generated the results in the first place, so it seems inelegant to call it again. So what I've done instead is to cache aliases in the popup when results are added, and then just look up the first word in the input in these aliases. We shouldn't reset this cache until the first result of a new search comes in. Differential Revision: https://phabricator.services.mozilla.com/D3850 --HG-- extra : moz-landing-system : lando --- browser/base/content/urlbarBindings.xml | 307 ++++++++++++++---------- 1 file changed, 184 insertions(+), 123 deletions(-) diff --git a/browser/base/content/urlbarBindings.xml b/browser/base/content/urlbarBindings.xml index 53984e06dc6e..17999324cae2 100644 --- a/browser/base/content/urlbarBindings.xml +++ b/browser/base/content/urlbarBindings.xml @@ -516,48 +516,76 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. true + false + + + + this[m]()); + ]]> + + + + + + + - + { // Check for re-entrance. On focus change this formatting code is // invoked regardless, thus this should be enough. - if (this._formattingInstance != instance) + if (this._formatURLInstance != instance) { return; + } let directionality = window.windowUtils.getDirectionFromText(domain); // In the future, for example in bug 525831, we may add a forceRTL // char just after the domain, and in such a case we should not @@ -617,10 +650,13 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. } }); - if (onlyEnsureFormattedHostVisible || !this._formattingEnabled) - return; + if (onlyEnsureFormattedHostVisible || !this._formattingEnabled) { + return false; + } - this.formatScheme(controller.SELECTION_URLSECONDARY); + let controller = this.editor.selectionController; + + this._formatScheme(controller.SELECTION_URLSECONDARY); // Strike out the "https" part if mixed active content is loaded. if (this.getAttribute("pageproxystate") == "valid" && @@ -630,8 +666,10 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. let range = document.createRange(); range.setStart(textNode, 0); range.setEnd(textNode, 5); + let strikeOut = + controller.getSelection(controller.SELECTION_URLSTRIKEOUT); strikeOut.addRange(range); - this.formatScheme(controller.SELECTION_URLSTRIKEOUT); + this._formatScheme(controller.SELECTION_URLSTRIKEOUT); } let baseDomain = domain; @@ -649,6 +687,9 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. subDomain = domain.slice(0, -baseDomain.length); } + let selection = + controller.getSelection(controller.SELECTION_URLSECONDARY); + let rangeLength = preDomain.length + subDomain.length - trimmedLength; if (rangeLength) { let range = document.createRange(); @@ -664,10 +705,12 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. range.setEnd(textNode, value.length - trimmedLength); selection.addRange(range); } + + return true; ]]> - + + + + + + + + + + { this._resizeThrottleTimeout = null; - this.formatValue(true); + this._formatURL(true); }, 100); } break; @@ -1796,82 +1917,6 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. this.controller.startSearch(value); ]]> - - - - - @@ -2621,15 +2666,31 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/. ]]> + + null +