Closed Bug 1517308 Opened 10 months ago Closed 9 months ago

Add a search field in about:url-classifier

Categories

(Toolkit :: Safe Browsing, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: baku, Assigned: baku)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

My idea is to add a search functionality which allows the user to check if a URL is stored in one of the url-classifier features. 

This is nice for debugging purposes.
Dimi, I'm going to as you to review this code after all the other patches.
Attached image screenshot.png
Here a screenshot of how it looks like.
(In reply to Andrea Marchesini [:baku] from comment #0)
> My idea is to add a search functionality which allows the user to check if a
> URL is stored in one of the url-classifier features. 
> This is nice for debugging purposes.

Great idea! Thanks for doing this!
Status: NEW → ASSIGNED
Priority: -- → P1
Attachment #9034003 - Flags: review?(dlee)
Attachment #9034004 - Flags: review?(dlee)
Comment on attachment 9034004 [details] [diff] [review]
part 2 - about page

Review of attachment 9034004 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Baku, thanks for doing this, I really like this feature!!!

::: toolkit/content/aboutUrlClassifier.js
@@ +113,5 @@
> +    }
> +
> +    let listType = document.getElementById("search-listtype").value == 0
> +                     ? Ci.nsIUrlClassifierFeature.blacklist
> +                     : Ci.nsIUrlClassifierFeature.whitelist;

I didn't test the code. But if I check the whitelist and also include LoginReputation feature checkbox, will the code fail at GetTables[1]?
If yes, I think we can just use Base::GetTables instead of MOZ_ASSERT in FeatureLoginReputation, return an empty string is good enough.

[1] https://searchfox.org/mozilla-central/rev/f8de61826903996f6bdf41b11a2844dd59ac144f/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#144
Attachment #9034004 - Flags: review?(dlee) → review+
Attachment #9034003 - Flags: review?(dlee) → review+

Comment on attachment 9034004 [details] [diff] [review]
part 2 - about page

Flod, can you take a look at the FTL file? Thanks.

Attachment #9034004 - Flags: review?(francesco.lodolo)

Comment on attachment 9034004 [details] [diff] [review]
part 2 - about page

Review of attachment 9034004 [details] [diff] [review]:

LGTM

P.S. On Phabricator I would have seen this automatically ;-)

Attachment #9034004 - Flags: review?(francesco.lodolo) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/543e146f314a
Add a search field in about:url-classifier - part 1 - nsIUrlClassifier.getFeatureNames(), r=dimi
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ab926764616
Add a search field in about:url-classifier - part 2 - about page, r=dimi, r=flod
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/268455a2f791
Add a search field in about:url-classifier - part 3 - ftl updated, r=flod
You need to log in before you can comment on or make changes to this bug.