Can't trigger update for google in about:url-classifier
Categories
(Toolkit :: Safe Browsing, defect, P1)
Tracking
()
People
(Reporter: obotisan, Assigned: dimi)
Details
Attachments
(1 file)
Affected versions
- ESR 60.6.0
- Latest Nightly 67.0a1
- Firefox 66.0
Affected platforms
- Ubuntu 18.04 x64
- Windows 10 x64
- macOS 10.14
Steps to reproduce
- Go to about:url-classifier
- Click on Trigger Update for google, in the Provider section.
Expected result
- The update is successful.
Actual result
- Nothing happens.
Regression range
- This is a regression. I can't reproduce the issue on builds from 2017-06-02. I will try to find more info as soon as possible.
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Reporter | ||
Comment 1•6 years ago
|
||
Regression range
- First bad: 2017-12-29 (20171229100308)
- Last good: 2017-12-30 (20171230220352)
- Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f0f170bd40e84daeae7eeb3f658a34d05a968d75&tochange=e02699fd3129bef049f848e2f71189182eabe555
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
This is because right now we don't have any feature uses SafeBrowsing V2 list, and even we have, we will still get an error because Google doesn't support V2 anymore.
Hi botisan,
are you ok with WONTFIX or you prefer an error message "cannot update"?
Reporter | ||
Comment 3•6 years ago
|
||
Ok... in this situation maybe is better that the error "cannot update" to be displayed. At least the button seems to be doing something.
But on the other hand, if Google doesn't support V2 anymore, why not just take that part out? Or do you plan to implement V2 back at some point?
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to Oana Botisan, Desktop Release QA from comment #3)
Ok... in this situation maybe is better that the error "cannot update" to be displayed. At least the button seems to be doing something.
But on the other hand, if Google doesn't support V2 anymore, why not just take that part out? Or do you plan to implement V2 back at some point?
Those fields are dynamically generated which is based on the data we have in the Preference.
And you are right, we can probably remove those Google V2 related stuff from preference, but since we have testcases still using Google V2 preference to test SafeBrowsing v2 protocol (we are still using v2 protocol for our tracking protection).
Remove them is not a trivial task to do and I don't really see too much benefit to doing it.
Assignee | ||
Comment 5•6 years ago
|
||
Pushed by dlee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c78da14598ce Remove provider from about:url-classifier if no table is being used. r=gcp
Assignee | ||
Comment 7•6 years ago
|
||
Hi Oana,
I fixed this bug by checking if there is any table being used for a given provider, if no, it won't show up in the about:url-classifier.
So in this case, "goog" will not show up.
Reporter | ||
Comment 8•6 years ago
|
||
Awesome. Thank you. Let me know when I can verify the issue on Nightly and beta so I can close the bug.
Comment 9•6 years ago
|
||
bugherder |
Hi Ethan (as Dimi is OOO), with 67 status as affected, should we consider uplifting this to Beta67? If yes, please nominate a patch for uplift. And the same question for ESR60.7.
Comment 11•6 years ago
|
||
If I understand comment 2 and comment 4 correctly, this is not a regression, and it's not necessary to uplift the patch.
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 12•6 years ago
|
||
Why would the Release mgmt bot keep setting the keyword regression? Automatically I guess?
Comment 13•6 years ago
|
||
The bot is adding the 'regression' keyword because the bug has a regression range. If it is not a regression, 'Has Regression Range' should not be set (or be set to 'irrelevant') and the 'regression' keyword should be removed.
That said, given that I) the button worked before and no longer works, and II) we do have a regression range, this seems to perfectly fit the definition of 'regression'?
Comment 14•6 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #13)
That said, given that I) the button worked before and no longer works, and II) we do have a regression range, this seems to perfectly fit the definition of 'regression'?
The button didn't work because Google doesn't support Safe Browsing V2, not due to our code defect.
The regression range just reflects the date we're moving from V2 to V4.
Comment 15•6 years ago
|
||
The button didn't work because Google doesn't support Safe Browsing V2, not due to our code defect.
At the end, from the user perspective, this is still a regression... Doesn't matter if this is our fault or not.
Comment 16•6 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
At the end, from the user perspective, this is still a regression... Doesn't matter if this is our fault or not.
That's fair. Thanks for providing the perspective. :)
But this is a minor issue. There's not much value to uplift the patch.
Updated•5 years ago
|
Comment 17•5 years ago
|
||
"Google" not show up anymore in the Provider section from about: url-classifier, I verified this in 68.0b7 on Win 10 x64, macOS 10.13.6 and Ubuntu 18.04 x64.
Description
•