The weather suggestion will be shown on zero prefix until remote settings are synced
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
People
(Reporter: adw, Assigned: adw)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
If remote settings and Nimbus don't define a weather config, Firefox will show the suggestion on zero prefix. That means if remote settings contain a config but it hasn't been synced yet, Firefox will go from showing the suggestion on zero prefix to showing it on keywords after settings are synced.
Firefox shouldn't show the suggestion at all if there's no config.
Assignee | ||
Comment 1•1 year ago
|
||
[Tracking Requested - why for this release]: This bug is necessary for the Firefox Suggest weather suggestion feature we intend to ship in 114.
Assignee | ||
Comment 2•1 year ago
|
||
This fixes the bug by not starting fetches until a config is set from either
remote settings or Nimbus. By "config" I mean keywords basically, but we sync
more than keywords -- the min keyword length and min keyword length cap -- so
that's why I use different term. So, before remote settings is synced, no config
will be set, so no fetches will happen, so the suggestion will be null. When the
urlbar provider starts a query, it will see the suggestion is null and not add a
result. Once a config is set from RS or Nimbus, we'll start fetching.
Currently we allow zero prefix when keywords or the min keyword length aren't
set. This patch removes that functionality because on second thought, there's
not a safe and obvious way to support zero prefix using these keyword-related
config properties/variables by themselves, and zero prefix isn't a feature
requirement anymore anyway. If we wanted to keep supporting it with these
properties/variables, there are a few options, and I don't like any of them:
- If
keywords
is undefined or null, use zero prefix. This is dangerous because
we may make a mistake in RS or Nimbus and forget to set it. Also, we use null
as the default value for the Nimbus var, and since we use UrlbarPrefs to
access Nimbus vars, there's no good way to tell whether null was set
intentionally or not. - If
keywords
is an empty array, use zero prefix. This is awkward because the
user can now increment the min keyword length, which means the keywords array
kept byWeather
can become empty when the min keyword length is incremented
a lot. In that case, no suggestion should be shown at all. - If
min_keyword_length
is zero/falsey, use zero prefix. This has the same
problems as the first point above.
If we do need to support zero prefix again in the future, I think we should add
an RS property/Nimbus variable that makes it clear what's happening, e.g., a
useZeroPrefix
boolean.
I removed the exposure scalar because it's entirely based on zero prefix. We can
use Glean for that now anyway.
I also noticed weather suggestions are case insenstive, so I fixed that and
added a test task.
Comment 4•1 year ago
|
||
Backed out for causing bc failures on browser_glean_telemetry_engagement_selected_result.js
Assignee | ||
Updated•1 year ago
|
Comment 6•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Assignee | ||
Comment 7•1 year ago
|
||
STR for QA
Please create a new profile and enable the weather suggestion and urlbar Debug logging. If you can inject the prefs into the profile before starting it so that the weather feature is enabled immediately when you do start it, that would be ideal.
browser.urlbar.weather.featureGate = true
browser.urlbar.weather.ignoreVPN = true
browser.urlbar.loglevel = Debug
- Start Firefox with the new profile
- Open the browser console
- Click in the urlbar several times and verify the weather suggestion is not shown. Before Firefox syncs remote settings in the next step, the suggestion should never be shown.
- Within about half a minute, Firefox should sync remote settings for the first time. To tell when this happens, filter on "quicksuggest.weather" in the browser console and you should see a message similar to this:
1683663568414 urlbar DEBUG QuickSuggest.Weather :: Got weather records: [{"type":"weather","schema":1683294047906,"weather":{"keywords":["weather","forecast","windy","humidity","monsoon","flooding","rain in","storms","storm in","forcast","wether","wather","weahter","weater","weaher","vindy"],"min_keyword_length":3,"min_keyword_length_cap":6},"id":"weather","last_modified":1683313881574}]
- Click in the urlbar again and verify the suggestion is not shown
- Type "weather" and verify the suggestion is shown
Assignee | ||
Comment 8•1 year ago
|
||
Comment on attachment 9332297 [details]
Bug 1831971 - Remove zero-prefix functionality from the weather suggestion and don't start fetches until a config is set.
Beta/Release Uplift Approval Request
- User impact if declined: This bug is necessary for the Firefox Suggest weather suggestion feature we intend to ship in 114.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: Please see comment 7
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This patch only affects the Firefox Suggest weather suggestion feature. Has automated tests.
- String changes made/needed:
- Is Android affected?: No
Comment 9•1 year ago
|
||
I have verified this issue on the latest Firefox Nightly 115.0a1 (Build ID: 20230510094206), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.
- The weather suggestion is no longer shown in the zero prefix panel.
- The weather suggestion is only triggered using specific keywords from Remote Settings.
- I confirm that using the STR from comment 7, I also got the following log in the Browser Console:
1683729000813 urlbar DEBUG QuickSuggest.Weather :: Got weather records: [{"type":"weather","schema":1683294047906,"weather":{"keywords":["weather","forecast","windy","humidity","monsoon","flooding","rain in","storms","storm in","forcast","wether","wather","weahter","weater","weaher","vindy"],"min_keyword_length":3,"min_keyword_length_cap":6},"id":"weather","last_modified":1683313881574}]
Comment 10•1 year ago
|
||
Comment on attachment 9332297 [details]
Bug 1831971 - Remove zero-prefix functionality from the weather suggestion and don't start fetches until a config is set.
Approved for 114 beta 3, thanks.
Comment 11•1 year ago
|
||
bugherder uplift |
Updated•1 year ago
|
Comment 12•1 year ago
|
||
I have verified this issue on the latest Firefox Beta 114.0b3 (Build ID: 20230511191645), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.
- The weather suggestion is no longer shown in the zero prefix panel.
- The weather suggestion is only triggered using specific keywords from Remote Settings.
- I confirm that using the STR from comment 7, I also got the following log in the Browser Console:
1683729000813 urlbar DEBUG QuickSuggest.Weather :: Got weather records: [{"type":"weather","schema":1683294047906,"weather":{"keywords":["weather","forecast","windy","humidity","monsoon","flooding","rain in","storms","storm in","forcast","wether","wather","weahter","weater","weaher","vindy"],"min_keyword_length":3,"min_keyword_length_cap":6},"id":"weather","last_modified":1683313881574}]
Description
•