This moves l10n strings related to AMP and Wikipedia suggestions out of
`enUS-searchFeatures.ftl` into the appropriate l10n files in preparation for AMP
and Wikipedia in non-U.S. regions. These strings include:
* The result menu item strings, Dismiss and Manage
* Relevant settings UI strings
`urlbar-result-menu-learn-more-about-firefox-suggest` isn't actually used by AMP
and Wikipedia right now, but it was in the past, and there have been recent
discussions about maybe including it again as Suggest expands outside the U.S.
So I moved it too in case we need it with short notice.
There are other Suggest strings that this patch does not move, in particular:
* `-firefox-suggest-brand-name` is already exposed to localizers
* The "Sponsored" label at the bottom of AMP urlbar rows is already exposed to
localizers as `urlbar-result-action-sponsored`
* Strings for the online toggle switch in the settings UI ("Improve the Firefox
Suggest experience") aren't needed right now because online Suggest (Merino)
won't be available outside the U.S. in the near future.
I changed the ID of the Dismiss string so it doesn't include "firefox-suggest".
Several non-Suggest urlbar results use this string too, and it doesn't actually
include the phrase "Firefox Suggest" anyway.
I also made the view default to this string so that dismissable urlbar results
don't need to specify it, similar to how it defaults to strings for "Learn more"
and Manage.
Depends on D238847
Differential Revision: https://phabricator.services.mozilla.com/D239213
When the view is updating existing rows and it encounters a hidden-exposure
result, it shouldn't increment `rowIndex` because the row isn't actually updated
in that case. The view should revisit the row in the next iteration.
Differential Revision: https://phabricator.services.mozilla.com/D238147
If the user is modifying their search term, we suspect they don't immediately
want an autofilled url. makeQueryContext always enables autofill if
the allowAutofill is undefined.
Differential Revision: https://phabricator.services.mozilla.com/D233396
This makes a number of changes to support a new UI treatment for weather
suggestions in addition to the two existing treatments. Unfortunately we need
all three to experiment with, but hopefully after experimentation is done we can
remove the two existing treatments.
This patch refers to the new treatment as the "simplest" UI. The Figma is at [1]
although it doesn't reflect some details Josh and I have discussed. One of the
existing treatments is already called "simple", and this patch changes its name
to "simpler". The patch refers to the other existing treatment as "full". These
names are only informal because the Nimbus variable and fallback pref are
integers.
Because the new UI treatment is so simple, we can pretty much use the standard
rich suggestion UI for it. The existing treatments are dynamic. I say "pretty
much" because I did need to add a capability to rich suggestions,
`result.richSuggestionIconVariation`, which lets results set an `icon-variation`
attribute on their rows. Weather suggestions use it to choose the appropriate
weather icon in the CSS.
I also modified l10n string caching in a few ways to address a couple of
problems:
* I like how dynamic results can cache their strings lazily by setting
`cacheable` in their l10n objects. Standard results can only cache their
strings by adding them to `UrlbarView.#cacheL10nStrings()`. That method is
getting pretty big, and it also doesn't work well with strings that have
arguments since it's called to pre-cache strings before they're used, when
there are no arguments. So I extended `cacheable` support to all l10n objects,
not only those from dynamic results.
* Josh's spec calls for the temperature in the suggestion title to be bolded.
There's no good way to do that right now. We have highlights, but they aren't
useable for l10n strings. The simplest thing -- and what I've done before in
cases like this -- is to include `<strong>` tags in the l10n string itself.
That works except for when we have a cache hit with the l10n cache. In that
case, we set the `textContent`, which of course renders the tag literally. So
I added a `parseMarkup` option to l10n objects that says the cached string
should be set as `innerHTML` instead. Actually, for security I parse the
string into a sanitized document fragment. We could just always do this and
get rid of `parseMarkup`, but it seems wasteful since most l10n strings don't
need it.
* While addressing the previous two points, it made sense to move
`setElementL10n()` from the view to `L10nCache`. That way all these
cache-related functions are together.
Finally, Josh noticed one of the dark-mode colors in the icon SVG didn't have
enough contrast with the standard dark-mode panel background, so I updated that
to a new color he chose (from `#80808F` to `#9393A8`).
[1] https://www.figma.com/design/Hdi0oHB7trRcncyVAKZypO/accuweather-explorations?node-id=3548-15992&t=f4YUShEe5RkP2KEg-1
Differential Revision: https://phabricator.services.mozilla.com/D232703
This patch adds English restrict keyword strings to the enUS-searchFeature.ftl
file. The restrictKeywords providers return an array of keywords for their
l10nRestrictKeywords property. The first is the localized keyword, and the
second is the English keyword. If the user is in the English locale, the
providers return an array with one element: the English keyword.
Differential Revision: https://phabricator.services.mozilla.com/D230009
This patch adds the keywords to the result title for TokenAliasEngine and
RestrictKeyword results. The TokenAliasEngine results may have more than one
keyword if the user has set a custom keyword for that engine. This can help
remind the user what keywords are available as they are typing or if they
have forgotten a keyword, they can type `@` to quickly see the list of keywords
to search with.
The result title looks like this:
For Engines:
@[keywords] - Search with [Engine]
For Restrict Keywords:
@[keyword] - Search [Restrict]
Differential Revision: https://phabricator.services.mozilla.com/D224017
The purpose of this change is to cache persist state into an object so that in future patches, I can add other persisted search related state to it.
Differential Revision: https://phabricator.services.mozilla.com/D224122
This replaces the current potential exposures implementation with "keyword
exposures." The current implementation is based on a keyword list defined in
Nimbus. Keyword exposures improve on the following drawbacks to that approach:
* Potential exposures can't be recorded for existing result types without
duplicating their keywords in the Nimbus list. The ability to record potential
exposures for existing types is an idea from DS (Dave).
* They're unrelated to `exposure` telemetry, so we don't get an `exposure` event
for them in the main ping, making it hard to correlate them with engagements
and abandonments. This is another drawback pointed out by DS (Dave). (By
design, we don't want to correlate individual keywords with
engagements/abandonments for privacy reasons, but we do want to know whether a
potential exposure was triggered during an engagement/abandonment without
knowing the matching keywords.)
* Storing the keywords in Nimbus means the Nimbus recipe is at least as large as
the keyword list. It's probably not a great idea to put thousands of keywords
in the recipe JSON.
* The keyword-matching strategy is simplistic exact matching. It can't do more
sophisticated strategies used for real results, like how we recently started
using FTS in Suggest.
Keyword exposures as implemented by this revision improve on all that. Summary:
* Keyword exposures are implemented on top of existing `exposure` telemetry.
They can be enabled for any result type, and they are always "in addition to"
`exposure` telemetry.
* They're enabled with a new bool Nimbus variable. When true, they're recorded
for all results that trigger `exposure` telemetry.
* They work with the new `Exposure` Rust suggestions (bug 1915317, bug 1893086,
result type "rust_exposure"). In combination, `Exposure` suggestions and
keyword exposures are the new way to do "potential exposures," i.e., exposure
telemetry for hypothetical suggestions that includes matching keywords. Like
every other Rust suggestion type, `Exposure` suggestions take their keywords
from remote settings and can do sophisticated matching.
Other changes in this revision:
* Keyword exposures are recorded on each instance of a matched result. With the
current approach, potential exposures are recorded per unique matched keyword.
That gives us more info. I cleared this idea with Dave.
* Properly detect `terminal` cases by comparing the final query context with the
context at the time of the exposure. The current detection is wrong because
it's only based on search strings, which doesn't work because the final search
string could also have been typed earlier in the session.
* In the feature manifest, change the branch of the related `setPref` variables
from `default` to `user`. When an experiment is uninstalled, the Nimbus client
[does *not* restore previous defaults](https://searchfox.org/mozilla-central/rev/446ff34da077a31d5550359480cb327f729c027b/toolkit/components/normandy/lib/PrefUtils.sys.mjs#119) for prefs set on the default branch. That
breaks some of the tests and also doesn't seem good for users.
Differential Revision: https://phabricator.services.mozilla.com/D220501
This replaces `UrlbarResult.exposureResultType` and `exposureResultHidden` with
a single simple value called `exposureTelemetry`. Please see the bug for the
rationale.
Other changes:
* For convenience, make `UrlbarPrefs.get("exposureResults")` return a `Set` so
consumers don't have to parse the string value.
* Simplify handling of result properties in `check_results()` (xpcshell tests)
* Add more tasks/checks to test_exposure.js
Depends on D219939
Differential Revision: https://phabricator.services.mozilla.com/D219942
This patch includes:
- A new UrlbarProviderRestrictKeywords class
- Showing localized Search with Bookmarks, Search with History, Search with Tabs results after the user types @
- Add search restrict keywords to preferences UI Search Shortcuts table
- Hiding search restrict keyword behind browser.urlbar.searchRestrictKeywords.featureGate pref
Differential Revision: https://phabricator.services.mozilla.com/D213697
There are a few ways to solve this problem and I tried to take a straightforward
approach. This patch makes text highlighting less automagical and surprising. It
forces dynamic-result providers to specify highlighted text content as part of
their view updates just like all other non-highlighted text and other
properties. To trigger highlighting, the view update can include a new
`highlights` property.
Except for Fakespot, there currently aren't any providers that take advantage of
the existing automagical highlighting behavior.
Depends on D216370
Differential Revision: https://phabricator.services.mozilla.com/D217020
The problem has to do with hidden exposures whose hypothetical rows can't
immediately be made visible either because they would overflow the filled-up
view or because a misplaced result was seen.
The relevant part of `#updateResults()` is [here](https://searchfox.org/mozilla-central/rev/6a2a2a52d7e544a2fd5678d04991a7e78b694f22/browser/components/urlbar/UrlbarView.sys.mjs#1289-1291). When `canBeVisible` is false,
we effectively forget about that hidden-exposure result, which is not right. For
normal results, we also have that `canBeVisible` logic (a few lines down from
the part I linked), but the difference is that we create a hidden row for them
and then unhide the row when the query finishes and stale rows are removed.
We're missing an analog to that logic for hidden-exposure results, so we're
undercounting them.
Fortunately it's not too hard to fix. I added a new kind of exposure for
`TelemetryEvent` to track called "tentative" exposures. Tentative exposures are
entirely analogous to this case where a row would be added but hidden, and then
unhidden when stale rows are removed.
While I was modifying the test, I rephrased the comments for existing tasks so
hopefully they're easier to understand.
Differential Revision: https://phabricator.services.mozilla.com/D205155
The heuristic result is always kept first when receiving new results, that means
any result could be upgraded to be a heuristic result, including the first Top
Site, as in the zero prefix case there's no heuritic.
Thus it's safer to generate new DOM contents when upgrading between heuristic and
non-heuristic.
Also switch-tab results with container info have complex structure and should
generate new DOM contents, unless they are reused by other switch-tab results.
Unfortunately these changes are mostly visual and not easily testable, as it would
be very time consuming.
In general, for the long term, reusing the DOM of results is fragile and causes
nodes, attributes or classes to be ported over to incompatible results. We do it
to avoid flicker, but we must investigate better ways to do it.
Differential Revision: https://phabricator.services.mozilla.com/D204483
