As we discussed, this adds a new observer service notification that Firefox
Suggest can send when it's done updating prefs as a result of a scenario change
and needs TelemetryEnvironment to re-cache them. That's only necessary on
startup when the scenario is set initially (as described in the bug), but this
revision does it unconditionally. That shouldn't be a problem because the only
other time the scenario changes is when Nimbus enrolls/unenrolls users, which
only happens at most twice -- or a small number of times maybe, if the user is
enrolled multiple times for some reason. The point is that it's a rare event and
not, like, multiple times a session.
Differential Revision: https://phabricator.services.mozilla.com/D126017
While this is mainly a refactoring patch, a few visible effects are expected:
- Proton Primary teal is now used correctly in dark mode. I confirmed with UX that we want this.
- Various minor elements now may use slightly different colors than before. Things like hover states and borders are now functions of the main colors on the page rather than their own hardcoded colors. While one might be able to spot differences in a side-by-side comparison, the general idea is to capture the same "feeling".
Differential Revision: https://phabricator.services.mozilla.com/D125607
This reworks the fix to bug 1729776.
Currently Nimbus doesn't have a way to force the two suggestions prefs [1] on or
off. That might seem surprising since we've run experiments already. Initially
we defaulted the two prefs to true but we defaulted the separate feature-gate
pref [2] to false, and it was the feature-gate pref we controlled via Nimbus. At
some point we changed the defaults to false and then in Firefox flipped them to
true after showing the onboarding dialog. As a result we've never needed to
override the two suggestion prefs via Nimbus.
The problem now is that we default-enable offline (for US en users), so we set
all three prefs to true. For the online rollout, we need to keep the
feature-gate pref enabled but disable the suggestion prefs, and there's no way
to do that.
My first idea was to add new Nimbus variables to override the two suggestion
prefs. The prefs would keep their default true values but be overridden by
Nimbus. But that doesn't work because there's no way for Firefox to tell whether
the prefs are true because the user has opted in (overriding Nimbus) or because
they still have their default values. Setting the prefs to true on the user
branch doesn't have any effect because they're also true on the default branch.
Or maybe there's a way I don't know about to force them to true on the user
branch, but even if there were, it seems brittle to rely on a value being set on
the user branch to distinguish between the two cases. (This is a potential
problem for any prefs that are controlled by both Nimbus and the user. So far
the prefs we've been using via Nimbus have all been hidden feature-gate-type
things and implementation details.)
We already have a `quickSuggestScenario` variable. We currently use it only to
tell what we should send in the telemetry ping (bug 1729576) and whether some
parts of the prefs UI should be shown. This revision makes it much more
important by treating it as the source of truth for the user's scenario. It now
determines the default values of related prefs, including the two suggestions
prefs.
The logic is:
```
If quickSuggestScenario is non-null:
scenario = quickSuggestScenario
Else (e.g., there's no rollout):
If the user is US en:
scenario = offline
Else:
scenario = history
```
After determining the scenario, it's set it as
`browser.urlbar.quicksuggest.scenario` on the default branch. There's an
existing pref observer in UrlbarPrefs, and I added a case for this pref so that
when it's updated we also update all the other prefs that depend on the
scenario. This way when the pref is set -- either due to Nimbus update or by
changing it on about:config -- all the other prefs stay in sync.
I kept the default value of `browser.urlbar.quicksuggest.scenario` but removed
it as the fallback for `quickSuggestScenario`. If it still both had a default
and remained the fallback, then it would be impossible to tell when Nimbus is
trying to override it, because any fetch of the value from Nimbus would just
return the fallback pref's value if there is no override.
I considered instead removing the default value and keeping it as the fallback.
The drawback of that is that unenrollments would not take effect until restart.
I actually tried this approach first, and in tests, after mock experiments were
unenrolled, the pref values remained what they were when the experiment was
active.
It might also be possible to not have the `browser.urlbar.quicksuggest.scenario`
pref at all. We could call NimbusFeatures directly to get the scenario. However,
currently we cache and access Nimbus variables through UrlbarPrefs, as we do
with prefs, and I don't want to add an inconsistency.
This revision also fixes bug 1730596 since it was easy to do given that I needed
a way to prevent indirect recursive updates to the scenario, and I can use that
for bug 1730596 too (the `_updatingFirefoxSuggestScenario` bool).
[1] `browser.urlbar.suggest.quicksuggest` and `browser.urlbar.suggest.quicksuggest.sponsored`
[2] `browser.urlbar.quicksuggest.enabled`
Differential Revision: https://phabricator.services.mozilla.com/D125511
This makes a couple of changes:
* Never discard quick suggest results. At some point we may want to revisit, but
right now we should always show them.
* Discard other URL results whose URLs dupe the URLs of quick suggest results or
whose URLs have a lower prefix rank.
The first change by itself fixes the bug, but without the second change, we'll
still show the eBay history result above the eBay quick suggest, which I
confirmed with Natalie is not what we want. We only want to show the quick
suggest.
Keep in mind that the results in this case have the same URLs, same prefixes.
When a non-quick-suggest result has a *higher* prefix rank, I think we should
*not* discard it. In that case we should show both the quick suggest with its
lower prefix rank and the other result with its higher rank.
Differential Revision: https://phabricator.services.mozilla.com/D125768
Give `tabindex` attributes to each focusable element and handle enter keypresses
(via the `dialogaccept` event) on them. The tab index ordering is:
1. Accept button (initial focus)
2. Settings button
3. Learn more
4. Not now
Also give `#infoBody` a tab index of -1 so it doesn't receive focus. It does
normally for some reason.
The accept button still does not have a focus ring initially. When you hit tab
the first time, the focus ring appear on the settings button. IMO it's better
not to show the ring initially since most people won't be interacting with the
dialog with the keyboard, and even those that do don't necessarily need to see
it initially. If we do want it initially, calling `focus` on the button in a
load event listener works.
On Windows (and maybe Linux?), the arrow keys still move around the elements
until the focus gets stuck on the Not now link. After that the arrow keys don't
work anymore but the tab key still does. Not sure how to fix that part.
Differential Revision: https://phabricator.services.mozilla.com/D125616
When switching to an already open but lazy/unloaded about:preferences tab and
highlighting a section in it, wait for it to load first.
Differential Revision: https://phabricator.services.mozilla.com/D125628
While the comment at the top of build-mingw32-nsis.sh is true, it only
applies to nsis being in the same path in the build tasks as it was
built from in the toolchain task. So we don't actually need to build
it in a mingw32/bin directory, and can ship it in a nsis/bin directory,
as long as that's where we build it.
That makes the toolchain match the expectations from bootstrap, which
also makes PATH adjustments for nsis unnecessary, no other toolchain
used in those builds providing a binary in mingw32/bin.
Differential Revision: https://phabricator.services.mozilla.com/D125637
BINDGEN_CFLAGS have not been necessary since bug 1526857.
The wine directory has not been necessary in $PATH since bug 1686646.
LD_LIBRARY_PATH adjustments for mingw32 were cargo culted and never
necessary AFAICT, and the ones for clang/lib have not been necessary
since bug 1690937.
Differential Revision: https://phabricator.services.mozilla.com/D125636
This patch introduces a new pref, devtools.every-frame-target.enabled,
that when set to true will cause target actors to be created on every
iframe, no matter if they are remote or not, no matter if Fission is
enabled or not.
This adds a ignoreSubFrames property on the BrowsingContextActor so
it can focus solely on the docShell it was passed.
Differential Revision: https://phabricator.services.mozilla.com/D125517
1. $LocalAppData behavior changes in NSIS 3.02, previously it always
used CSIDL_LOCAL_APPDATA but it now depends on context, work around
that by directly calling SHGetSpecialFolderPathW.
2. Refactor several other calls to SHGetSpecialFolderPathW for
CSIDL_COMMON_APPDATA and CSIDL_PROGRAMS.
3. Remove broken default path fallback to $APPDATA. I was in this
code for 1. and realized it hadn't worked properly in the full
installer since bug 367539, and it must have never worked in the stub.
4. Remove unused CleanUpdateDirectories and DeleteRelativeProfiles
macros rather than trying to fix them.
Differential Revision: https://phabricator.services.mozilla.com/D125490
Drew Willcoxon
Dan Mosedale
Thomas Wisniewski
Erik Nordin
Andrei Oprea
Neil Deakin
Harry Twyford
Narcis Beleuzu
Florian Quèze
Itiel
Dão Gottwald
Punam Dahiya
Tim Huang
Daisuke Akatsuka
Scott
Niklas Baumgardner
Butkovits Atila
Micah Tigley
Michael Kohler
Marian-Vasile Laza
Bob Owen
Mike Hommey
Nicolas Chevobbe
Andreea Pavel
Adam Gashlin
Cristian Tuns
Hanna Jones
Katherine Patenio