1996203 - The AMP and Wikipedia results are not triggered on native locales in DE, FR and IT

Status: NEW

(Firefox :: Address Bar, defect, P3)

Description

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Attachment: firefox suggest EU not working.mp4

[Affected versions]:

• Firefox Release 144
• FIrefox Beta 145.0b5
• Firefox Nightly 146.0a1

[Affected Platforms]:

• Windows 10 x64
• macOS 14.4.1
• Linux Mint 20.1

[Prerequisites]:

• Have Firefox Release 144.0 it build extracted/installed.
• Make sure that the browser.search.region is set to IT
• Have the Nimbus Developer Tools addon installed.
  • Using this add-on, from the Production environment and force enroll in the following:
    • First in the: Firefox Suggest Geo Expansion - EU re-ramp with amp-and-wikipedia-suggestions branch
    • Second in the: Firefox Suggest Geo Expansion Firefox 141+ with amp-and-wikipedia-suggestions branch

[Steps to reproduce]:

1. Open the Firefox browser with the profile from prerequisites.
2. Type a keyword that should trigger an AMP result (eg: nike presto women, canotta basket, groupon, vestito da golf donna, etc).
3. Observe the Firefox Suggest results.

[Expected result]:

• The AMP result is correctly triggered.

[Actual result]:

• No AMP result is triggered.

[Notes]:

• The issue is also reproducible for Wikipedia results.
• I have also tried with a VPN connection to IT, but still no suggestions were triggered.
• I have also tested with DE and FR regions.
• Attached is a screen recording of the issues.

Comment 1

[Drew Willcoxon :adw]

Thanks Cosmin. The video is very helpful and captures everything I need, so I appreciate that.

I was able to reproduce your video as much as I could tell: forced enrollment in both the experiment and rollout, IT locale and region. I used 144.0.

It looks like this is a real problem in the urlbar code, in UrlbarPrefs. I ran this in the browser console:

NimbusFeatures.urlbar.getVariable("ampFeatureGate")
// => true
UrlbarPrefs.get("ampFeatureGate")
// => false

These two calls should always return the same value. Urlbar thinks AMP is disabled, but Nimbus correctly says it's enabled. I think the problem is related to having both the experiment and the rollout installed at once. UrlbarPrefs has a NimbusFeatures.urlbar.onUpdate that caches all Nimbus variables. That caching mechanism has never been a problem before, and looking at it now, it looks correct to me. So I think what's happening is that Nimbus is calling the update callback for the old experiment after the new rollout. The new rollout defines ampFeatureGate: true but the old experiment doesn't define that variable, so it ends up not being present in urlbar's cache.

I'll need to look into it.

Comment 2

[Drew Willcoxon :adw]

Attachment: Browser console screenshot

Oh, NimbusFeatures.urlbar.getAllVariables() returns an object with ampFeatureGate: false! That's the problem. That's what urlbar uses to cache its Nimbus variables. NimbusFeatures.urlbar.getVariable("ampFeatureGate") returns true, getAllVariables() returns { ampFeatureGate: false }. The correct value is true -- at least it seems like it should be.

I haven't ingestigated this any more than that, but I'll move this bug to Nimbus for now. Depending on the outcome, we may (also) want to modify urlbar to use getVariable().

Comment 3

[Drew Willcoxon :adw]

NI'ing Jared per our Slack conversation.

Comment 4

[Beth Rennie [:beth] (she/her)]

Hi! getvariable() and getVariables() operate slightly differently:

function getAllVariables(defaultValues) {
    values = experiment ? experiment.values : rollout.values ?? {};
    return { ...fallbackPrefs, ...defaultValues, ...values }
}

function getVariable(v) {
    return experiment.values[v] ?? rollout.values[v] ?? fallbackPrefs[v]
}

source

If we look at the experiment and rollout payloads, we see the rollout has ampFeatureGate: true but ampFeatureGate is not in the experiment payload. Thus getAllVariables() will not see it and return the default value provided by the fallbackPref.

Comment 5

[Drew Willcoxon :adw]

Thanks Beth, I see. That's pretty surprising, if I could offer some feedback as an API consumer, so much so that it seems like a bug to me. I think it's reasonable to expect the two functions to return the same values given their names.

I'll move this back to Address Bar and file another bug for replacing getAllVariables with getVariable within urlbar.

Comment 6

[Drew Willcoxon :adw]

I filed bug 1996973 for using getVariable instead of getAllVariables. I'll downgrade the priority of this bug because the latest plan for the EU is to stop the current experiment and rollout and do an entirely new rollout by itself, which will avoid this bug. We should probably just close it at that time. I'll leave it open until then.