Details
- Reviewers
twisniewski timhuang denschub smaug - Group Reviewers
anti-tracking-reviewers Restricted Project webidl - Commits
- Restricted Diffusion Commit
rMOZILLACENTRAL303cf334fa0d: Bug 1638383 - Add a shim for the dFPI Microsoft login breakage. r=anti-tracking… - Bugzilla Bug ID
- 1638383
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable - Build Status
Buildable 374887 Build 469647: arc lint + arc unit
Event Timeline
This still needs a warning logged to the console to make users / developers aware that we're shimming. Could be in both teams / powerva and login.microsoftonline.com.
Also, we don't really do anything useful if the user denies the rSA prompt. But I'm afraid there isn't much we can do if we don't want to edit web content.
Would just issuing a window.alert be a reasonable compromise there, letting the user know login will fail and they may need to hit the back button?
This would work, but I'm afraid that content prompts are too closely associated with coming from the site. We could show a tab-prompt, which looks more like chrome UI. This would require extending the trackingProtection.js interface to support calls to Services.prompt.*. Maybe a nice follow-up.
The part of changing privilege rSA looks good to me.
I am wondering if we could have some sort of test here for the redirect shimming mechanism. But maybe we don't have a test infrastructure for this I guess.
I agree that it would be great to have test coverage for this. This is challenging due to the nature of shims. I've added an agenda item for our next meeting.
Added web console warnings. Derive requestStorageAccessOrigin from redirect, instead of passing it explicitly.
This revision requires a Testing Policy Project Tag to be set before landing. Please apply one of testing-approved, testing-exception-unchanged, testing-exception-ui, testing-exception-elsewhere, testing-exception-other. Tip: this Firefox add-on makes it easy!
Sorry to jump on this, but this is lacking a version-number bump in browser/extensions/webcompat/manifest.json before landing. Unfortunately, that's required to invalidate caches etc.
Also note that there currently is another patch, D131738, in-flight and that's already on autoland, but not yet in central. So this patch should set 28.6.0, and it's probably smart to rebase this on top of the other patch as soon as that's merged, to avoid conflicts. :)
We consume user activation before entering the async bits of requestStorageAccessForOrigin. This ment the previous version would never auto-grant. Making the auto grant decision on the initial user activation state fixes this.
@timhuang could you please have another look?
Ah, right. Sorry I didn't catch this in the first place. The change looks good to me. And thanks for adding the test.
requestStorageAccessForOrigin has test coverage. For the shims we're still working on a test environment.
The following parameter set has differences:
It can be previewed here for one week.
If you see a problem in this automated review, please report it here.