Skip to content

Commit

Permalink
Backed out changeset 6e3078121a13 (bug 1657676) for bc failures at br…
Browse files Browse the repository at this point in the history
…owser_oneOffs_keyModifiers.js. CLOSED TREE
  • Loading branch information
cristianbrindusan committed Sep 16, 2020
1 parent 7249c61 commit 09fafce
Show file tree
Hide file tree
Showing 13 changed files with 32 additions and 270 deletions.
6 changes: 2 additions & 4 deletions browser/base/content/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4135,11 +4135,9 @@ const BrowserSearch = {
if (delayUpdate && !gURLBar.value) {
// Delays changing the URL Bar placeholder until the user is not going to be
// seeing it, e.g. when there is a value entered in the bar, or if there is
// a tab switch to a tab which has a url loaded. We delay the update until
// the user is out of search mode since an alternative placeholder is used
// in search mode.
// a tab switch to a tab which has a url loaded.
let placeholderUpdateListener = () => {
if (gURLBar.value && !gURLBar.searchMode) {
if (gURLBar.value) {
// By the time the user has switched, they may have changed the engine
// again, so we need to call this function again but with the
// new engine name.
Expand Down
4 changes: 1 addition & 3 deletions browser/components/urlbar/UrlbarController.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,7 @@ class UrlbarController {
break;
case KeyEvent.DOM_VK_RIGHT:
case KeyEvent.DOM_VK_END:
this.input.maybePromoteKeywordToSearchMode({
entry: "typed",
});
this.input.maybePromoteKeywordToSearchMode();
// Fall through.
case KeyEvent.DOM_VK_LEFT:
case KeyEvent.DOM_VK_HOME:
Expand Down
87 changes: 17 additions & 70 deletions browser/components/urlbar/UrlbarInput.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -938,15 +938,6 @@ class UrlbarInput {
// confusing the user, we always enforce it when a result changes our value.
this.setPageProxyState("invalid", true);

// A previous result may have previewed search mode. If we don't expect that
// we might stay in a search mode of some kind, exit it now.
if (
this.searchMode?.isPreview &&
result?.payload.keywordOffer != UrlbarUtils.KEYWORD_OFFER.SHOW
) {
this.setSearchMode({});
}

if (!result) {
// This happens when there's no selection, for example when moving to the
// one-offs search settings button, or to the input field when Top Sites
Expand All @@ -966,20 +957,6 @@ class UrlbarInput {
} else if (result.autofill) {
let { value, selectionStart, selectionEnd } = result.autofill;
this._autofillValue(value, selectionStart, selectionEnd);
} else if (
result.payload.keywordOffer == UrlbarUtils.KEYWORD_OFFER.SHOW
) {
// Enter search mode without starting a query. This will just preview
// search mode.
let enteredSearchMode = this.maybePromoteKeywordToSearchMode({
result,
checkValue: false,
startQuery: false,
});
if (!enteredSearchMode) {
this._setValue(this._getValueFromResult(result), true);
this.setSearchMode({});
}
} else {
// If the url is trimmed but it's invalid (for example it has an unknown
// single word host, or an unknown domain suffix), trimming
Expand Down Expand Up @@ -1069,11 +1046,7 @@ class UrlbarInput {
firstResult.heuristic &&
firstResult.payload.keyword &&
!firstResult.payload.keywordOffer &&
this.maybePromoteKeywordToSearchMode({
result: firstResult,
entry: "typed",
checkValue: false,
})
this.maybePromoteKeywordToSearchMode(firstResult, false)
) {
return true;
}
Expand Down Expand Up @@ -1172,18 +1145,6 @@ class UrlbarInput {
};

if (this.searchMode) {
// The user started a search from search mode preview so we're now in
// regular search mode. We `delete` instead of setting to null for easier
// testing. If this is our first query for this search mode, we record
// search mode entry in telemetry.
if (this.searchMode.isPreview) {
delete this.searchMode.isPreview;
try {
BrowserUsageTelemetry.recordSearchMode(this.searchMode);
} catch (ex) {
Cu.reportError(ex);
}
}
options.searchMode = this.searchMode;
if (this.searchMode.source) {
options.sources = [this.searchMode.source];
Expand Down Expand Up @@ -1384,10 +1345,6 @@ class UrlbarInput {
} else {
this.searchMode.entry = entry;
}

// We always preview search mode before entering it. We enter full search
// mode once a query is started.
this.searchMode.isPreview = true;
this.toggleAttribute("searchmode", true);
// Clear autofill.
if (this._autofillPlaceholder && this.window.gBrowser.userTypedValue) {
Expand All @@ -1402,6 +1359,13 @@ class UrlbarInput {
this.window.gBrowser.selectedBrowser,
this.searchMode
);

// Record search mode entry in telemetry.
try {
BrowserUsageTelemetry.recordSearchMode(this.searchMode);
} catch (ex) {
Cu.reportError(ex);
}
} else {
this.removeAttribute("searchmode");
this._searchModesByBrowser.delete(this.window.gBrowser.selectedBrowser);
Expand Down Expand Up @@ -1607,41 +1571,30 @@ class UrlbarInput {
* Enters search mode and starts a new search if appropriate for the given
* result. See also _searchModeForResult.
*
* @param {string} entry
* The search mode entry point. See setSearchMode documentation for details.
* @param {UrlbarResult} [result]
* The result to promote. Defaults to the currently selected result.
* @param {boolean} [checkValue]
* @param {UrlbarResult} result
* The currently selected urlbar result.
* @param {boolean} checkValue
* If true, the trimmed input value must equal the result's keyword in order
* to enter search mode.
* @param {boolean} [startQuery]
* If true, start a query after entering search mode. Defaults to true.
* @returns {boolean}
* True if we entered search mode and false if not.
*/
maybePromoteKeywordToSearchMode({
entry,
maybePromoteKeywordToSearchMode(
result = this._resultForCurrentValue,
checkValue = true,
startQuery = true,
}) {
if (
!result ||
(checkValue && this.value.trim() != result.payload.keyword?.trim())
) {
checkValue = true
) {
if (checkValue && this.value.trim() != result.payload.keyword?.trim()) {
return false;
}

let searchMode = this._searchModeForResult(result, entry);
let searchMode = this._searchModeForResult(result, "typed");
if (!searchMode) {
return false;
}

this.setSearchMode(searchMode);
this._setValue(result.payload.query?.trimStart() || "", false);
if (startQuery) {
this.startQuery({ allowAutofill: false });
}
this.startQuery({ allowAutofill: false });
return true;
}

Expand Down Expand Up @@ -2490,12 +2443,6 @@ class UrlbarInput {
}
this._focusUntrimmedValue = null;

// We exit previewed search mode on blur since the result previewing it is
// implictly unselected.
if (this.searchMode?.isPreview) {
this.setSearchMode({});
}

this.formatValue();
this._resetSearchState();

Expand Down
2 changes: 1 addition & 1 deletion browser/components/urlbar/UrlbarProviderTopSites.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class ProviderTopSites extends UrlbarProvider {
...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, {
title: site.title,
keyword: site.title,
keywordOffer: UrlbarUtils.KEYWORD_OFFER.SHOW,
keywordOffer: UrlbarUtils.KEYWORD_OFFER.HIDE,
engine: engine.name,
query: "",
icon: site.favicon,
Expand Down
5 changes: 1 addition & 4 deletions browser/components/urlbar/UrlbarResult.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ class UrlbarResult {
case UrlbarUtils.RESULT_TYPE.SEARCH:
switch (this.payload.keywordOffer) {
case UrlbarUtils.KEYWORD_OFFER.SHOW:
if (!UrlbarPrefs.get("update2")) {
return [this.payload.keyword, this.payloadHighlights.keyword];
}
// Fall through.
return [this.payload.keyword, this.payloadHighlights.keyword];
case UrlbarUtils.KEYWORD_OFFER.HIDE:
return ["", []];
}
Expand Down
6 changes: 3 additions & 3 deletions browser/components/urlbar/UrlbarUtils.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ var UrlbarUtils = {
// when the user picks them. Depending on the use case, a keyword offer can
// visually show or hide the keyword itself in its result. For example,
// typing "@" by itself will show keyword offers for all engines with @
// aliases, and those results will preview their search modes. When a keyword
// offer is a heuristic -- like an autofilled @ alias -- usually it hides
// its keyword since the user is already typing it.
// aliases, and those results will visually show their keywords -- @google,
// @bing, etc. When a keyword offer is a heuristic -- like an autofilled @
// alias -- usually it hides its keyword since the user is already typing it.
KEYWORD_OFFER: {
SHOW: 1,
HIDE: 2,
Expand Down
8 changes: 1 addition & 7 deletions browser/components/urlbar/tests/UrlbarTestUtils.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -481,13 +481,7 @@ var UrlbarTestUtils = {

// If this is an engine search mode, check that all results are either
// search results with the same engine or have the same host as the engine.
// Search mode preview can show other results since it is not supposed to
// start a query.
if (
expectedSearchMode.engineName &&
!expectedSearchMode.isPreview &&
this.isPopupOpen(window)
) {
if (expectedSearchMode.engineName && this.isPopupOpen(window)) {
let resultCount = this.getResultCount(window);
for (let i = 0; i < resultCount; i++) {
let result = await this.getDetailsOfResultAt(window, i);
Expand Down
1 change: 0 additions & 1 deletion browser/components/urlbar/tests/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ support-files =
[browser_searchMode_engineRemoval.js]
[browser_searchMode_no_results.js]
[browser_searchMode_pickResult.js]
[browser_searchMode_preview.js]
[browser_searchMode_setURI.js]
[browser_searchMode_suggestions.js]
support-files =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,7 @@
"use strict";

add_task(async function test() {
for (let i = 0; i < 5; i++) {
await PlacesTestUtils.addVisits("http://example.com/");
}
// Update Top Sites to make sure the last Top Site is a URL. Otherwise, it
// would be a search shortcut and thus would not fill the Urlbar when
// selected.
await updateTopSites(sites => {
return (
sites &&
sites[sites.length - 1] &&
sites[sites.length - 1].url == "http://example.com/"
);
});

await PlacesTestUtils.addVisits("http://example.com/");
registerCleanupFunction(async function() {
await PlacesUtils.history.clear();
});
Expand Down
Loading

0 comments on commit 09fafce

Please sign in to comment.