Closed Bug 1557153 Opened 6 years ago Closed 5 years ago

UITour default permissions overridden by UNKNOWN_ACTION [Skyline]

Categories

(Firefox :: Tours, defect, P1)

69 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- wontfix
firefox70 + verified
firefox71 --- verified

People

(Reporter: hoosteeno, Assigned: Mardak)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [skyline])

Attachments

(3 files, 1 obsolete file)

Some users (two, so far, internal to Mozilla) on nightly are not getting the expected behavior from UITour. But not all users.

The symptoms of this bug are that Firefox users visiting web pages that implement UITour do not get UITour behavior on those pages. For example:

In the developer console, running Mozilla.Client.getFxaDetails(function(details){console.log(details)}) while visiting any of the above URLs should return an object with UITour status.

  • Users who report this issue are signed in, according to the browser UI, but that object incorrectly includes a setup: false property.
  • Users who report this issue have devices set up, according to the Accounts setup page, but that object incorrectly includes false in the desktopDevices and mobileDevices properties.

Some users reporting the issue are on MacOS with buildid 20190605095225, but other users on the same OS with the same buildid cannot replicate the issue.

Users reporting the issue are unable to replicate in DevEdition or Release.

This issue will be very impactful if it gets to release, because it will prevent critical Trailhead marketing flows from working correctly.

Does the issue happen in safe mode for the affected users? Is privacy.resistFingerprinting on? Output from Mozilla.Client.FirefoxDetails on the web console for affected users would also be good.

Flags: needinfo?(hoosteeno)

Restarted in safe mode (which updated me to buildid 20190605215957),
https://www.mozilla.org/en-US/firefox/63.0/tracking-protection/start/?variation=0&step=1 still doesn't show the tour,
privacy.resistFingerprinting is false,
Mozilla.Client.FirefoxDetails is Object { accurate: false, version: "69.0", channel: "release", distribution: undefined, isUpToDate: true, isESR: false }.

A couple of notes to help debug here:

  • Mozilla.Client.FirefoxDetails is a bedrock wrapper around the UITour API. Returning accurate: false indicates that UITour is not enabled for a user. However, to test if this is a problem with UITour, or a problem in bedrock, it may be better to suggest another snippet of code that is a little "closer to the metal":

    1. Open https://www.mozilla.org/en-US/firefox/63.0/tracking-protection/start/?variation=0&step=1
    2. Open the Web Console
    3. Paste the following code and obeserve the response:
    Mozilla.UITour.getConfiguration('sync', function (config) {
        console.log(config);
    });
    

    The result of the console.log should return a users signed-in state for Sync. If UITour is not working, there may be no output returned at all.

  • It's also worth noting that a user should only see a tour at the /tracking-protection/ URL if they have TP enabled. People only get sent to this page once a tracker is detected, and the tour relies on the TP feature being active in order to work.

I get undefined when I run that code. I also get undefined when I try to get the configuration for 'appinfo' and 'availableTargets', and 'jfdljfsd' (the last of which I would have expected to throw some sort of error)…
If I open up the Browser Toolbox, I see NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface] stack-trace-collector.js:75, which might be helpful?
And I believe I have enabled tracking protection.

Ok, that sounds like it is indeed an issue with UITour, since the callback is never returning.

Ooh, more interesting data…
I signed out, and then into my bwinton@mozilla.com sync account, and it worked just fine.
Then I signed out and back into my bwinton@latte.ca sync account, and it worked once, but is now failing again.
One thing that bit us previously is that my sync name is Blake Winton ✨☕️✨, so if your stack isn't handling emojis, that could be related…

Flags: needinfo?(MattN+bmo)

(In reply to Blake Winton (:bwinton) (:☕️) from comment #6)

One thing that bit us previously is that my sync name is Blake Winton ✨☕️✨, so if your stack isn't handling emojis, that could be related…

👀

Do you data from GA to indicate how many users would be affected by this? Without being able to reproduce it's unlikely that we'll work on a fix since there are higher priority projects going on. Do we have someone else who can still reproduce?

I think we have no idea what causes some users to get no UITour in nightly. If we knew, we might be able to estimate the impact on the release population.

Not getting UITour will cause some troublesome experiences on WMO and SUMO, including on the whatsnew page. If the impacts are felt by a large population, this could be problematic enough to invoke a fire drill. But, like I said, I think we don't have any way to guess how it unfolds, because the bug is not understood.

Flags: needinfo?(hoosteeno)

I can still reproduce it, and am happy to run whatever debugging y'all think may help. 🙂

Adding [Skyline] to summary and a few observers for visibility, as recommended in #skyline-engage-model Slack channel as a stopgap until Skyline triage process is defined.

Summary: UITour seems broken in recent nightlies → UITour seems broken in recent nightlies [Skyline]

(In reply to Blake Winton (:bwinton) (:☕️) from comment #10)

I can still reproduce it, and am happy to run whatever debugging y'all think may help. 🙂

Oh, I misread comment 6 to say that you couldn't reproduce anymore so that's why this went quiet. Would you mind testing if this affects Fx70 beta to see if it will actually be a problem for Skyline? Maybe copy your profile and allow a downgrade to test with the exact same profile data.

Fx70 Nightly with the old profile fails.
Fx69 Beta with a new profile seems to work.
Fx69 Beta with a downgraded old profile fails.

Blake, setting browser.uitour.loglevel to "Debug" would provide more logging in the Browser Console which you could attach and maybe something will jump out. If someone wants to use a debugger on UITour.jsm it should be fairly clear what's going on.

I still won't have time to look at this for another week or so… I kinda think UITour should fall under the scope of the User Journey team.

Keywords: regression

bwinton, for the failing profile, what's the value for services.sync.username (when UITour is failing)? And just making sure, the toolbar Firefox Account icon is the logged in or not logged in icon?

Also, in your profile directory, is there a signedInUser.json file with a json .accountData.email that matches? Perhaps also .accountData.profileCache.profile.email too? If you want to share the structure + emails, run this in your browser console:

(async () => {
  console.log(
    JSON.stringify( 
      JSON.parse(
        await OS.File.read(
          OS.Path.join(OS.Constants.Path.profileDir, "signedInUser.json"),
          { encoding: "utf-8" }
        )
      ),
      null,
      2
    ).replace(/("(?!email).*": ").*"/g, `$1…"`)
  );
})();

should have some output like:

{
  "version": 1,
  "accountData": {
    "email": "bwinton@latte.ca",
    "sessionToken": "…",
    "uid": "…",
    "verified": true,
    "device": {
      "sendTabKeys": {
        "publicKey": "…",
        "privateKey": {
          "crv": "…",
          "d": "…",
          "ext": true,
          "key_ops": [
            "deriveBits"
          ],
          "kty": "…",
          "x": "…",
          "y": "…"
        },
        "authSecret": "…"
      },
      "id": "…",
      "registrationVersion": 2,
      "registeredCommandsKeys": [
        "https://identity.mozilla.com/cmd/open-uri"
      ]
    },
    "oauthTokens": {
      "profile": {
        "token": "…",
        "server": "…"
      }
    },
    "profileCache": {
      "profile": {
        "email": "bwinton@latte.ca",
        "locale": "…",
        "amrValues": [
          "pwd",
          "email"
        ],
        "twoFactorAuthentication": false,
        "uid": "…",
        "avatar": "…",
        "avatarDefault": false,
        "displayName": "…"
      },
      "etag": "…"
    }
  }
}

Separately, I tried setting display name to Blake Winton ✨☕️✨ and doesn't seem enough to cause problems for me.

Flags: needinfo?(bwinton)

(In reply to Blake Winton (:bwinton) (:☕️) from comment #2)

https://www.mozilla.org/en-US/firefox/63.0/tracking-protection/start/?variation=0&step=1 still doesn't show the tour,

Tour for tracking-protection shouldn't be used for testing this bug at least on nightly as it's been removed in bug 1564367.

See Also: → 1564367

bwinton, here's an example of setting browser.uitour.loglevel = Debug and visiting https://www.mozilla.org/en-US/firefox/accounts/

There should be two sets of onPageEvent (2 object to expand) followed by a sendPageCallback (1 object) where the one is configuration: "sync" and other "appinfo"

Blake are you taking this bug? If not -- Kate, can you help find an owner? Should this be whiteboard tagged as [uj]?

Flags: needinfo?(khudson)
Priority: -- → P1

I'm definitely not fixing it, but I'll try to provide info to help other people debug it as I can.

Flags: needinfo?(bwinton)

If you're still running into the bug, can you provide a screenshot as in comment 17

Flags: needinfo?(bwinton)

(In reply to Ed Lee :Mardak from comment #15)

bwinton, for the failing profile, what's the value for services.sync.username (when UITour is failing)?

bwinton@latte.ca

And just making sure, the toolbar Firefox Account icon is the logged in or not logged in icon?

I removed the account icon from the toolbar, but the one in the hamburger menu is logged in, and if I put it back, it's logged in.

Also, in your profile directory, is there a signedInUser.json file with a json .accountData.email that matches?

Yep.

Perhaps also .accountData.profileCache.profile.email too?

Also the same.

If you want to share the structure + emails, run this in your browser console:

{
  "version": 1,
  "accountData": {
    "email": "bwinton@latte.ca",
    "sessionToken": "…",
    "uid": "…",
    "verified": true,
    "device": {
      "sendTabKeys": {
        "publicKey": "…",
        "privateKey": {
          "crv": "…",
          "d": "…",
          "ext": true,
          "key_ops": [
            "deriveBits"
          ],
          "kty": "…",
          "x": "…",
          "y": "…"
        },
        "authSecret": "…"
      },
      "id": "…",
      "registrationVersion": 2,
      "registeredCommandsKeys": [
        "https://identity.mozilla.com/cmd/open-uri"
      ]
    },
    "oauthTokens": {
      "profile": {
        "token": "…",
        "server": "…"
      }
    },
    "profileCache": {
      "profile": {
        "email": "bwinton@latte.ca",
        "locale": "…",
        "amrValues": [
          "pwd",
          "email",
          "otp"
        ],
        "twoFactorAuthentication": true,
        "uid": "…",
        "avatar": "…",
        "avatarDefault": false,
        "displayName": "…"
      },
      "etag": "…"
    }
  }
}

Finally, I had nothing in the debug log when visiting https://www.mozilla.org/en-US/firefox/accounts/
(If you want a screenshot of an empty Browser Toolbox window, I'll attach one, but it's just a big expanse of white pixels. 😉)

Flags: needinfo?(bwinton)

(In reply to Blake Winton (:bwinton) (:☕️) from comment #21)

Finally, I had nothing in the debug log when visiting https://www.mozilla.org/en-US/firefox/accounts/

Mmm.... quick sanity check that you do see something similar to the screenshot with the account that does work correctly with browser.uitour.loglevel = Debug? Otherwise, the empty browser console seems to suggest UITour.jsm isn't even getting the page event, so something broke in the message passing ??

(In addition to all the email and pref checks in comment 21 seems to indicate if UITour was responding to the page content's request, it would indicate the user is signed in -> show Monitor not sign up)

bwinton@latte.ca in DevEdition I see the debug messages.
bwinton@mozilla.com in Nightly, I get a blank browser console.

After investigating bwinton's build it looks like Services.perms.testPermission is returning 0 / UNKNOWN_ACTION some reason instead of 1 / ALLOW_ACTION inside UITourChild.jsm so the event is never getting through. When I get a chance we can dig into it more, but does that ring a bell for anyone?

Flags: needinfo?(khudson)

Hmm… I was worried about it being a permission issue. Are you using a container tab by any chance?

The permissions should be loaded via https://searchfox.org/mozilla-central/rev/1dfd70469212ef2785d41827c5532c571c784227/browser/app/permissions#9-14 and that mechanism was specifically made so that permissions wouldn't be lost if the permission manager DB was cleared…

Blake, can you run the following in your Browser Console and paste/attach the result:

Services.perms.getAllWithTypePrefix("uitour").map(perm => perm.capability + ": " + perm.principal.origin)
Flags: needinfo?(MattN+bmo) → needinfo?(bwinton)

Note that I updated the comment 25 code snippet. Can you also run the following from the Browser Console when the tour tab is selected:

gBrowser.selectedBrowser.contentPrincipal.origin

I did a little more debugging, and ran sqlite3 ./permissions.sqlite in the Nightly profile's directory, and select * from moz_perms where type == "uitour"; gave me:

407|https://www.mozilla.org|uitour|0|0|0|1541077196841
409|https://support.mozilla.org|uitour|0|0|0|1541077196857
411|https://screenshots.firefox.com|uitour|0|0|0|1541077196887
412|about:home|uitour|0|0|0|1541077196896
413|https://addons.mozilla.org|uitour|0|0|0|1541077196931
415|https://discovery.addons.mozilla.org|uitour|0|0|0|1541077196961
417|about:newtab|uitour|0|0|0|1541077196973

in my DevEdition profile (which works), the same command returns no rows.

In fact, there are no permissions with a value of "0" in my DevEdition Profile, as opposed to my Nightly profile, which has the following:

407|https://www.mozilla.org|uitour|0|0|0|1541077196841
408|https://testpilot.firefox.com|install|0|0|0|1541077196849
409|https://support.mozilla.org|uitour|0|0|0|1541077196857
410|https://support.mozilla.org|remote-troubleshooting|0|0|0|1541077196857
411|https://screenshots.firefox.com|uitour|0|0|0|1541077196887
412|about:home|uitour|0|0|0|1541077196896
413|https://addons.mozilla.org|uitour|0|0|0|1541077196931
414|https://addons.mozilla.org|install|0|0|0|1541077196931
415|https://discovery.addons.mozilla.org|uitour|0|0|0|1541077196961
416|https://discovery.addons.mozilla.org|hc_telemetry|0|0|0|1541077196961
417|about:newtab|uitour|0|0|0|1541077196973
418|https://input.mozilla.org|remote-troubleshooting|0|0|0|1541077196987

No container tabs.

gBrowser.selectedBrowser.contentPrincipal.origin is
"https://www.mozilla.org" in both Nightly and DevEdition.

Services.perms.getAllWithTypePrefix("uitour").map(perm => perm.capability + ": " + perm.principal.origin) is
Array [] in Nightly, and
Array(5) [ "1: https://www.mozilla.org", "1: https://support.mozilla.org", "1: https://screenshots.firefox.com", "1: about:home", "1: about:newtab" ] in DevEdition.

Flags: needinfo?(bwinton)

(In reply to Blake Winton (:bwinton) (:☕️) from comment #27)

I did a little more debugging, and ran sqlite3 ./permissions.sqlite in the Nightly profile's directory, and select * from moz_perms where type == "uitour"; gave me:

407|https://www.mozilla.org|uitour|0|0|0|1541077196841
…
in my DevEdition profile (which works), the same command returns no rows.

That is the normal behaviour btw. since permissions in browser/app/permissions should not be written to disk. The only time a row for uitour should be written to disk is if the user explicitly removes the permission but I'm guessing some code has done this for you by enumerating permissions or there was a Firefox bug at some point that got you into this bad state.

In fact, there are no permissions with a value of "0" in my DevEdition Profile, as opposed to my Nightly profile, which has the following:

407|https://www.mozilla.org|uitour|0|0|0|1541077196841
…

This user permission of UNKNOWN_ACTION = 0 is overriding the default permission of ALLOW_ACTION = 1;. I don't know that we are ever supposed to be writing 0s to storage. Let me remind myself how the code is supposed to work. Bug 506446 is what implemented reading from the permissions file.

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #29)

This user permission of UNKNOWN_ACTION = 0 is overriding the default permission of ALLOW_ACTION = 1;. I don't know that we are ever supposed to be writing 0s to storage.

If I understand this comment correctly then it seems like we are supposed to write UNKNOWN_ACTION = 0 to storage. I guess we need to figure out what consumer caused this and what API they used.

Do/did you have any extensions that would clear permissions?

I've had the profile for a while, so I don't know for sure, but I don't think so.
The add-ons I currently have are listed at https://cl.ly/9b793126c960 if any of them jump out.
I remember testing UITour stuff while it was being implemented, so perhaps something happened there which led to a slightly different value in a pref, which led to the clearing of the permission? (But that's a wild-ass guess at best, and doesn't explain the other person who saw the problem.)

(In reply to Blake Winton (:bwinton) (:☕️) from comment #27)

415|https://discovery.addons.mozilla.org|uitour|0|0|0|1541077196961

Curiously, all these UNKNOWN_ACTION = 0 permission overrides are the ones that were defaults in the permissions file where bug 1529952 removed the default permission for addons/discovery.addons back on 2019-02-23.

411|https://screenshots.firefox.com|uitour|0|0|0|1541077196887

screenshots was added back on 2017-09-08

And the timestamps on each of those permissions are Thu Nov 01 2018 05:59:56 GMT-0700 (Pacific Daylight Time) which does happen to fall between the time screenshots was added and addons removed.

So… something got installed 9am eastern time back on november 1st -- perhaps an extension or a bad update? ??? Would have been nightly 65 then.

ehsan, any idea what might have caused all of the default permissions (from https://searchfox.org/mozilla-central/source/browser/app/permissions ) to get set as UNKNOWN_ACTION around the end of october and potentially fixed for other users soon after? Totally confused, but curious if this is affecting more people in worse ways than just uitour getting blocked.

(In reply to Blake Winton (:bwinton) (:☕️) from comment #27)

407|https://www.mozilla.org|uitour|0|0|0|1541077196841
408|https://testpilot.firefox.com|install|0|0|0|1541077196849
409|https://support.mozilla.org|uitour|0|0|0|1541077196857
410|https://support.mozilla.org|remote-troubleshooting|0|0|0|1541077196857
411|https://screenshots.firefox.com|uitour|0|0|0|1541077196887
412|about:home|uitour|0|0|0|1541077196896
413|https://addons.mozilla.org|uitour|0|0|0|1541077196931
414|https://addons.mozilla.org|install|0|0|0|1541077196931
415|https://discovery.addons.mozilla.org|uitour|0|0|0|1541077196961
416|https://discovery.addons.mozilla.org|hc_telemetry|0|0|0|1541077196961
417|about:newtab|uitour|0|0|0|1541077196973
418|https://input.mozilla.org|remote-troubleshooting|0|0|0|1541077196987
Flags: needinfo?(ehsan)

(In reply to Ed Lee :Mardak from comment #33)

ehsan, any idea what might have caused all of the default permissions (from https://searchfox.org/mozilla-central/source/browser/app/permissions ) to get set as UNKNOWN_ACTION around the end of october and potentially fixed for other users soon after? Totally confused, but curious if this is affecting more people in worse ways than just uitour getting blocked.

Hmm, interesting. That's a very hard question to answer from memory. :-) That file is imported in a custom way into the permission manager and I remember that various code changes have broken that import in the past a few times...

I think what you want is to provide a detailed set of steps to reproduce that would allow detecting this problem either directly or through one of its manifestations, and then set the regressionwindow-wanted keyword on the bug to allow someone either from QA or the community to look at the bug and try to bisect it to see what was the exact changeset that caused the import of the default permissions to break.

To be honest I haven't followed all of the discussions on the bug and I'm not sure if this failure to import is the root cause of the bug (which would mean the steps to reproduce would basically be the STR for the bug), but clear STRs would really help whoever ends up doing the bisection for you.

Flags: needinfo?(ehsan)

Adding regression-window wanted per Ehsan's comment.

bwinton, if you can reproduce this, could you get a regression window for this bug

Flags: needinfo?(bwinton)

I can't reproduce the breaking of the profile, cause my profile is already broken, so I'm not going to be much help in getting a regression window, I'm afraid. (I can totally help test fixes, though, if we decide to patch the permissions!)

Flags: needinfo?(bwinton)

So… ha. I have a relatively low usage profile that I use exclusively on the beta channel and I was doing some quick tests and was surprised when I saw:

Services.perms.testPermission(Services.io.newURI("https://www.mozilla.org"), "uitour")
0
SELECT * FROM moz_perms WHERE type='uitour';
2|https://discovery.addons.mozilla.org|uitour|0|0|0|1509039190996
3|https://addons.mozilla.org|uitour|0|0|0|1509039190997
8|https://support.mozilla.org|uitour|0|0|0|1517519112262
19|https://www.mozilla.org|uitour|0|0|0|1562667225858

And those dates:

1509039190996,Thu Oct 26 2017 10:33:10 GMT-0700 (Pacific Daylight Time)
1509039190997,Thu Oct 26 2017 10:33:10 GMT-0700 (Pacific Daylight Time)
1517519112262,Thu Feb 01 2018 13:05:12 GMT-0800 (Pacific Standard Time)
1562667225858,Tue Jul 09 2019 03:13:45 GMT-0700 (Pacific Daylight Time)

I don't recall doing anything special re: permissions or uitour in this profile, and the dates for when the permission is set to UNKNOWN show that this has been happening for at nearly 2 years and is still happening as recently as last month. Curiously, that july 9th date is firefox 68 release date, so I might have upgraded to beta 69 then. Feb 1 2018 is a week after release 58. And Oct 26 2017 is close to quantum launch. Although I don't actively try to keep this profile on the latest betas.

Is there any way to add some (temporary) telemetry to see how many beta 70 or even nightly 71 users might be affected? (from the permissions you mention in comment 38)?

Flags: needinfo?(edilee)

Perhaps ignoring the desire for telemetry as from comment 38, this issue seems to have been around since 2017…

markh, would it be reasonable to not override the browser/app/permissions defaults perhaps maybe limiting to uitour as I don't think users have a way to turn it off anyway.


But if we do want telemetry before changing things, I see bug 1453667 removed telemetry from UITour but perhaps these permission override issues don't need to use firefox telemetry. We should be able to detect from mozilla.org pages whether UITour is available or not. And pretty much all Firefox users should have it available as I don't think users have any UI to turn the permission off.

E.g., on the whatsnew page, there's a "Congrats! You’re using the latest version of Firefox." message that can be shown:
https://github.com/mozilla/bedrock/blob/57f3915528f7293dcae4964380fdc024e2a2dd1c/media/js/firefox/whatsnew/up-to-date.js#L21-L22

Mozilla.UITour.ping(function() {
        checkUpToDate();

Where firefox with UNKNOWN_ACTION override will never respond to the ping. We could maybe have bedrock have a ping-timed-out to report back the failure.

Alternatively, I do see nsPermissionManager.cpp has various existing telemetry histograms where maybe we can report back how many uitour UNKNOWN entries there are ?

Flags: needinfo?(edilee) → needinfo?(markh)
See Also: → 1453667
Summary: UITour seems broken in recent nightlies [Skyline] → UITour default permissions overridden by UNKNOWN_ACTION [Skyline]

(In reply to Ed Lee :Mardak from comment #40)

markh, would it be reasonable to not override the browser/app/permissions defaults perhaps maybe limiting to uitour as I don't think users have a way to turn it off anyway.

This was an intentional design to allow enterprises to allow revoking of the permissions on those sites. Mike, do enterprise policies allow providing default permissions still? If you're fine with not allowing enterprises to revoke the UITour permissions for our built-in list then it would simplify things and prevent this bug for the future. We would then be able to do a migration to delete the old overrides.

But if we do want telemetry before changing things, I see bug 1453667 removed telemetry from UITour but perhaps these permission override issues don't need to use firefox telemetry. We should be able to detect from mozilla.org pages whether UITour is available or not. And pretty much all Firefox users should have it available as I don't think users have any UI to turn the permission off.

Right, I suggested this in comment 8 but it didn't get implemented on the website yet. I really think this is the faster/better way to measure this.

Given that this is an old regression I'm not sure we need to track it for Skyline anymore?

Flags: needinfo?(mozilla)

This was an intentional design to allow enterprises to allow revoking of the permissions on those sites. Mike, do enterprise policies allow providing default permissions still? If you're fine with not allowing enterprises to revoke the UITour permissions for our built-in list then it would simplify things and prevent this bug for the future. We would then be able to do a migration to delete the old overrides.

Enterprise policies use a unique temporary permission that was created for policies:

https://searchfox.org/mozilla-central/source/browser/components/enterprisepolicies/Policies.jsm#1476

And there is no mechanism for removing policies.

We also don't provide any policies around UITour.

So I'm, fine with not allowing enteprises to revoke UITour

Flags: needinfo?(mozilla)

Great, I only suggested telemetry out of wanting to try to measure impact (since this bug seemed kind of stalled, but is worrisome for Skyline release). It's an older regression but won't that surface more now with the skyline release? If you are confident that it won't, please go ahead and downgrade the priority and mark it wontfix for 70.

Note I touched the permission manager stuff once, roughly 5 years ago, so please take this with a grain of salt!

IIUC, the only real way for UNKNOWN to get in the DB is when removing the permission for a site that has a default. Explicitly calling Services.perms.add() with UNKNOWN for any other URL just removes any rows that might exist. However:

(In reply to Ed Lee :Mardak from comment #38)

SELECT * FROM moz_perms WHERE type='uitour';
2|https://discovery.addons.mozilla.org|uitour|0|0|0|1509039190996
3|https://addons.mozilla.org|uitour|0|0|0|1509039190997
8|https://support.mozilla.org|uitour|0|0|0|1517519112262
19|https://www.mozilla.org|uitour|0|0|0|1562667225858

What's odd here is that https://discovery.addons.mozilla.org does appear in browser/app/permissions but with the hc_telemetry permission. https://addons.mozilla.org doesn't appear at all. So it's not clear to me how Services.perms could be convinced to end up with a DB in that state given those sites never had uitour as a default permission.

Regardless:
(In reply to Ed Lee :Mardak from comment #40)

markh, would it be reasonable to not override the browser/app/permissions defaults perhaps maybe limiting to uitour as I don't think users have a way to turn it off anyway.

As already noted above, that would seem fine. It's not clear how to best implement that though - deleting all 'uitour' rows from the DB?

Flags: needinfo?(markh)

(In reply to Mark Hammond [:markh] from comment #44)

(In reply to Ed Lee :Mardak from comment #38)

SELECT * FROM moz_perms WHERE type='uitour';
2|https://discovery.addons.mozilla.org|uitour|0|0|0|1509039190996
3|https://addons.mozilla.org|uitour|0|0|0|1509039190997
8|https://support.mozilla.org|uitour|0|0|0|1517519112262
19|https://www.mozilla.org|uitour|0|0|0|1562667225858

What's odd here is that https://discovery.addons.mozilla.org does appear in browser/app/permissions but with the hc_telemetry permission. https://addons.mozilla.org doesn't appear at all. So it's not clear to me how Services.perms could be convinced to end up with a DB in that state given those sites never had uitour as a default permission.

It was added to the list on 2016-04-20 but later removed.

To be clear, this seems to affect all default permissions and not just uitour, e.g., bottom of comment 27 and here's the 0s from my same profile from comment 38:

SELECT * FROM moz_perms WHERE permission=0;
2|https://discovery.addons.mozilla.org|uitour|0|0|0|1509039190996
3|https://addons.mozilla.org|uitour|0|0|0|1509039190997
4|https://addons.mozilla.org|install|0|0|0|1509039190997
8|https://support.mozilla.org|uitour|0|0|0|1517519112262
9|https://support.mozilla.org|remote-troubleshooting|0|0|0|1517519112264
18|https://discovery.addons.mozilla.org|hc_telemetry|0|0|0|1562667219121
19|https://www.mozilla.org|uitour|0|0|0|1562667225858
27|https://screenshots.firefox.com|uitour|0|0|0|1565902376423

Not sure why bwinton's profile had everything set to UNKNOWN roughly at the same time whereas my profile seems to have affected a couple domains at a time.

Also very suspicious that the last entry for screenshots happened at Thu Aug 15 2019 13:52:56 GMT-0700 (Pacific Daylight Time) just few hours after I made comment 38. According to the update history, one didn't happen that day, so I guess we can rule out update-triggered bug (?):

Firefox 69.0 Beta 14 (20190815163925)
Installed on: August 16, 2019, 10:46:05 AM
Status: The Update was successfully installed

Firefox 69.0 Beta 13 (20190812173625)
Installed on: August 14, 2019, 12:56:22 PM
Status: The Update was successfully installed

I'll followup with bedrock to see if it's feasible to add some UITour-broken telemetry perhaps to whatsnew69 page.

A narrow fix / one-liner to uplift to 70 could be to modify UITourChild.ensureTrustedOrigin to just return true for mozilla.org similar to how chrome uris are allowed:
https://searchfox.org/mozilla-central/rev/2c0a60b5b065ae0fdfcfae168141e82b834fb2d4/browser/components/uitour/UITourChild.jsm#79

Flags: needinfo?(edilee)

Filed bedrock https://github.com/mozilla/bedrock/issues/7671 agibson thinks we should be able to get this data pretty quickly.

agibson did a quick sanity check with existing data based on how many Firefox desktop users get the graceful degradation behavior if Firefox UITour does not respond within a default 400ms:

5% of firefox users

This should be an upperbound estimate as it also includes people on slow machines that is competing for other things happening when first starting Firefox after an upgrade.


For reference, on fast machine (e.g., my development laptop):

(async() => {
  const o = [];
  for (let i = 0; i < 1000; i++) {
    const s = performance.now();
    await new Promise(r => Mozilla.UITour.ping(r));
    o.push(performance.now() - s);
  }
  console.log(o.filter(v => !v).length, o.sort((a, b) => b - a));
})()

780 Array(1000) [ 9, 8, 2, 1, 1, 1, 1, 1, 1, 1, … ]

That says out of 1000 UITour.ping requests, 780 pings returned in the same millisecond, 2 pings returned in 9 or 8 milliseconds, 1 ping in 2ms, and the remaining 217 pings in 1ms.

And running from a virtual machine using up resources both in the vm and host, more than half of the pings are still back within the same millisecond, and all are responded within 50ms.

546 Array(1000) [ 41, 10, 9, 6, 5, 5, 5, 4, 4, 4, … ]

So even the 400ms timeout with 5% should be relatively accurate, but we'll have better numbers with more direct UITour failure telemetry. And with a narrow fix to always allow https://www.mozilla.org in 70, hopefully we'll see immediate changes in the 400ms timeout and new telemetry.

Assignee: nobody → edilee
Flags: needinfo?(edilee)

Ha. ha. ha… As I was setting up a test profile to simulate a UNKNOWN uitour permission, I think I might have figured out what might be one of the causes. As from comment 44:

Explicitly calling Services.perms.add() with UNKNOWN for any other URL just removes any rows that might exist.

Services.perms.testPermission(Services.io.newURI("https://www.mozilla.org/"), "uitour")
1

Services.perms.remove(Services.io.newURI("https://www.mozilla.org/"), "uitour")
undefined

Services.perms.testPermission(Services.io.newURI("https://www.mozilla.org/"), "uitour")
0

So removing a default permission that wasn't actually in sqlite results in the permission immediately getting removed. So this got me wondering what mass removal would cause persisting UNKNOWN, and sure enough Clear Recent History with Site Preferences selected results in UNKNOWN saving to disk on shutdown. Some reason Services.perms.removeAllSince allows the default permissions to keep working in the current running firefox, but after restart, the UNKNOWN entries are used instead of defaults.

Looks like this has existed at least since moz_perms was introduced in Firefox 42 with bug 1185343. I grabbed https://ftp.mozilla.org/pub/firefox/releases/42.0/mac/en-US/Firefox%2042.0.dmg and cleared recent history + site preferences… sure enough after quitting firefox, moz_perms:

1|https://www.mozilla.org|uitour|0|0|0|1567791224358
2|https://support.mozilla.org|uitour|0|0|0|1567791224359
3|https://support.mozilla.org|remote-troubleshooting|0|0|0|1567791224359
4|https://marketplace.firefox.com|install|0|0|0|1567791224359
5|moz-safe-about:home|uitour|0|0|0|1567791224359
6|https://addons.mozilla.org|install|0|0|0|1567791224359
7|https://self-repair.mozilla.org|uitour|0|0|0|1567791224359
8|https://input.mozilla.org|remote-troubleshooting|0|0|0|1567791224359
See Also: → 1185343

Looks like bug 1058442 (firefox 36) introduced the call to pm.removeAllSince which is one of the user facing ways to trigger this UNKNOWN permission. The bug in removeAllSince might have always existed… bisecting some more.

Regressed by: 1058442
Regressed by: 1058433

Specially permit www.mozilla.org after ensuring other origin checks but failing ALLOW_ACTION.

See Also: → 1579517
Pushed by elee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/de62729b366c Workaround UITour default permissions overridden by UNKNOWN_ACTION r=MattN

After talking to Ed I’m confident the workaround will resolve any issues for 70 and we can prioritize the full fix as it makes sense.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

STR:

  1. create new profile
  2. open https://www.mozilla.org/firefox/69.0/whatsnew/
  3. see at top "Congrats! You’re using the latest version of Firefox."
  4. select menu History -> Clear Recent History…
  5. click (make sure checked) "Site Preferences" then click "Clear Now"
    a. maybe restart firefox
  6. open new tab to https://www.mozilla.org/firefox/69.0/whatsnew/

expected: see "Congrats! …" again

actual: no message at top

Flags: needinfo?(mcoman)
Flags: needinfo?(cmuresan)

I tried to verify this issue following the steps provided in Comment 57 on Latest Firefox Nightly 71.0a1 (Build ID: 20190908214439) on Windows 10 x64, Mac 10.14.5 and Arch Linux 4.16.6 x64.
The "Congrats!.." message is displayed again on the https://www.mozilla.org/firefox/69.0/whatsnew/ page after clearing the recent history with the Site Preferences option checked.

I have also tried to reproduce the initial issue but without any success on Firefox Nightly 69.0a1 (Build ID: 20190605095225) on 10 new profiles on Mac 10.14.5, 5 profiles on Windows 10 x64 and 2 profiles on Arch Linux 4.16.6 x64:

Due to the fact that I was not able also reproduce the initial issue, I cannot mark this issue as verified fixed.

Please let me know if I can help with anything else.

Flags: needinfo?(mcoman)
Flags: needinfo?(cmuresan)

Hmm… I tried with mac 20190605095225 and the back window is step 3 of comment 57 and the front (focused) window is after step 6 both showing https://www.mozilla.org/firefox/69.0/whatsnew/

To be clear, old versions should show the initial "Congrats! …" in step 3 and only after clearing "Site Preferences" and a new tab/window results in the missing message.

Perhaps as an extra step, try restarting Firefox after clearing Site Preferences before opening the whatsnew url ?

The other pages you tested while signed in to FxA should behave as if you were not logged in to FxA when the bug is active (i.e., clearing "Site Preferences" then opening a new tab). Although specially noting the tracking-protection tour will not work in 70 as it was removed in bug 1564367 https://www.mozilla.org/en-US/firefox/63.0/tracking-protection/start/?variation=0&step=1

Flags: needinfo?(acupsa)

Thanks Ed for the help provided. If I restart the browser after clearing "Site Preferences" I can manage to reproduce the initial issues on Firefox Build ID: 20190605095225.

I have reverified this issue following the steps provided in Commnent 57 and a restart of the browser after Step 5 on Latest Firefox Nightly 71.0a1 (Build ID: 20190909214621) with 5 profiles for each: Windows 10 x64, Mac 10.14.5 and Arch Linux 4.16.6 x64:

Status: RESOLVED → VERIFIED
Flags: needinfo?(acupsa)

Comment on attachment 9091138 [details]
Bug 1557153 - Workaround UITour default permissions overridden by UNKNOWN_ACTION

Beta/Release Uplift Approval Request

  • User impact if declined: ~5% of users see the wrong mozilla.org messaging that depends on UITour APIs
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 57
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small change to specially ignore permission manager and allow www.mozilla.org in UITour code
  • String changes made/needed: none
Attachment #9091138 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9091138 [details]
Bug 1557153 - Workaround UITour default permissions overridden by UNKNOWN_ACTION

Workaround for a Skyline issue, let's take this for 70.
Can you open a new bug please for a non-workaround fix if you intend one for 71 or beyond, and link to it from this bug? Thanks!

Flags: needinfo?(edilee)
Attachment #9091138 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Liz Henry (:lizzard) from comment #62)

Can you open a new bug please for a non-workaround fix if you intend one for 71 or beyond, and link to it from this bug? Thanks!

Yup filed bug 1579517

Blocks: 1579517
Flags: needinfo?(edilee)

I have verified this issue on Firefox Beta 70.0b6 (Build ID: 20190912160217) on Windows 10 x64, Mac 10.14.5 and Arch Linux 4.16.6 x64:

Following up on the bedrock telemetry that went to production a couple days ago on the 69.0/whatsnew page:

https://github.com/mozilla/bedrock/commit/8a224ce9da52fec64e90c2633ccb37533739b311

Initial numbers from jgmize seems to indicate 1.9% of pings are exceeding the 1 second ping timeout including those who never respond -> wrong UITour permission.

Compared to comment 48, which estimated about 5% of pings exceeding 400ms timeout, this smaller number should more accurately count the users that have the wrong permission instead of including users who have very slow machines.

Although there is some bias in that because we're collecting this on the 69/whatsnew page, these users are those who did not update immediately.

See Also: → 1593777
Has Regression Range: --- → yes
Regressions: 1752229

Hey pbz, I think this might be related to that uitour issue on SUMO we talked about a little while back.

(In reply to Mike Conley (:mconley) (:⚙️) from comment #67)

Hey pbz, I think this might be related to that uitour issue on SUMO we talked about a little while back.

I think so too. I'll try to get Bug 1579517 prioritized. Thanks for raising!

Comment on attachment 9338064 [details]
Bug 1557153 - Workaround UITour default permissions overridden by UNKNOWN_ACTION for SUMO. r?Mardak!

Revision D180328 was moved to bug 1837407. Setting attachment 9338064 [details] to obsolete.

Attachment #9338064 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: