Can't trigger update for google in about:url-classifier

VERIFIED FIXED in Firefox 68

Status

()

defect
P1
normal
VERIFIED FIXED
3 months ago
13 days ago

People

(Reporter: oana.botisan, Assigned: dimi)

Tracking

Trunk
mozilla68
Points:
---

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox65 wontfix, firefox66 wontfix, firefox67 wontfix, firefox68 verified)

Details

Attachments

(1 attachment)

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

  1. Go to about:url-classifier
  2. 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

3 months ago
Has Regression Range: --- → no
Has STR: --- → yes
Reporter

Comment 1

3 months ago

Regression range

Has Regression Range: no → yes
Assignee

Updated

3 months ago
Priority: -- → P2
Assignee

Updated

3 months ago
Priority: P2 → P1
Assignee

Updated

3 months ago
Assignee: nobody → dlee
Status: NEW → ASSIGNED
Assignee

Comment 2

3 months 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"?

Flags: needinfo?(oana.botisan)
Reporter

Comment 3

3 months 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?

Flags: needinfo?(oana.botisan)
Assignee

Comment 4

3 months 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.

Comment 6

3 months 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

3 months 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

3 months 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

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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.

Flags: needinfo?(ettseng)

If I understand comment 2 and comment 4 correctly, this is not a regression, and it's not necessary to uplift the patch.

Flags: needinfo?(ettseng)
Keywords: regression

Why would the Release mgmt bot keep setting the keyword regression? Automatically I guess?

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'?

(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.

Has Regression Range: yes → irrelevant
Keywords: regression

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.

(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.

Flags: qe-verify+

"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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.