The `urlclassifier.trackingAnnotationSkipURLs` pref do not accept some rules with uppercase characters
Categories
(Core :: Privacy: Anti-Tracking, defect)
Tracking
()
People
(Reporter: englehardt, Assigned: ehsan.akhgari)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
In Bug 1536898 we tested a temporary fix for the breakage described in Bug 1531818.
We tried using accounts.google.com/ServiceLogin
as a skip url, given that the current recaptcha skip url includes a path. The breakage was due to the lack of cookies on the url: https://accounts.google.com/ServiceLogin?passive=1209600&osid=1&continue=https://smartlock.google.com/client?callback%3DonGoogleYoloLoad&followup=https://smartlock.google.com/client?callback%3DonGoogleYoloLoad&authuser=0
.
Expected result: Breakage described in Bug 1531818 was fixed.
Actual result: The breakage was not fixed, and we instead needed to use accounts.google.com
as the skip url.
Although we didn't test it, we suspect this will also apply to the url-classifier-skip-urls
remote settings approach to adding skip urls.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
This regression is caused by bug 1511436 part 3. That patch removed a call to ToLowerCase() here: https://hg.mozilla.org/mozilla-central/rev/0cc5e8d1a09d#l19.195 and didn't replace it with anything. The problem mentioned in comment 0 is caused because of the upper case characters in accounts.google.com/ServiceLogin
.
Assignee | ||
Comment 2•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
Comment on attachment 9055636 [details]
Bug 1538974 - Ensure that we accept uppercase characters for url-classifier annotation skip URLs;
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1511436
- User impact if declined: We may not be able to push site-specific workarounds for Enhanced Tracking Protection to users successfully in order to address site breakage when we discover such problems. We can of course be careful to push such settings with all lower-case strings but it's easy to make human errors there and there won't be anything to stop us.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): A one liner change to a JS file.
- String changes made/needed: None
Comment 5•6 years ago
|
||
bugherder |
Updated•6 years ago
|
Updated•6 years ago
|
Comment 6•6 years ago
|
||
Comment on attachment 9055636 [details]
Bug 1538974 - Ensure that we accept uppercase characters for url-classifier annotation skip URLs;
Tracked 67 regression, well understood and low-risk patch with tests, uplift approved for 67 beta 9, thanks!
Comment 7•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Description
•