[SeaMonkey] test_classifier.html and test_classifier_worker.html fail

VERIFIED FIXED in Firefox 14

Status

()

Toolkit
Safe Browsing
P2
major
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 14
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox11 wontfix, firefox12 wontfix, firefox13 wontfix, firefox-esr10 wontfix)

Details

(Whiteboard: [perma-orange], URL)

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1332863004.1332865068.15862.gz
WINNT 5.2 comm-central-trunk debug test mochitests-5/5 on 2012/03/27 08:43:24
{
9670 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier.html | Should not load bad javascript - got loaded malware javascript!, expected untouched
9671 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier.html | Should not load bad css - didn't expect hidden, but got it

9674 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier_worker.html | failed to block evilWorker.js - got failure, expected success
9675 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier_worker.html | failed to block importScripts('evilWorker.js') - got failure, expected success
}

Bug 738883 packages url-classifier files.
Bug 738884 enables MOZ_URL_CLASSIFIER=1.

Now the related test are enabled :-) but fail :-(
Would they miss to set browser.safebrowsing.malware.enabled preference?

Comment 1

5 years ago
http://mxr.mozilla.org/comm-central/search?string=safebrowsing.malware.enabled

> Would they miss to set browser.safebrowsing.malware.enabled preference?
Possibly.

The unit tests but not the mochitests do the following:
// Enable malware/phishing checking for tests
prefBranch.setBoolPref("browser.safebrowsing.malware.enabled", true);
prefBranch.setBoolPref("browser.safebrowsing.enabled", true);

Comment 2

5 years ago
On the other hand we might have to implement the SafeBrowsing service in order for this to work. None of these test pages trigger anything in SeaMonkey do they?

http://www.mozilla.org/firefox/its-a-trap.html
http://www.mozilla.org/firefox/its-an-attack.html
(from http://www.mozilla.org/en-US/firefox/phishing-protection/)
(Assignee)

Comment 3

5 years ago
(In reply to Philip Chee from comment #1)
> > Would they miss to set browser.safebrowsing.malware.enabled preference?
> Possibly.

Confirmed by testing with Firefox.


(In reply to Philip Chee from comment #2)
> On the other hand we might have to implement the SafeBrowsing service in
> order for this to work.

If that were true, we should document it.
Assignee: nobody → sgautherie.bz
Blocks: 441359, 738884
Status: NEW → ASSIGNED
No longer depends on: 441359, 738884
Hardware: x86 → All
(Assignee)

Comment 4

5 years ago
Created attachment 610140 [details] [diff] [review]
(Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation
[Checked in: Comment 8]

What about "XXX How is this part of the test supposed to work (= be checked)?" in classifierFrame.html ?
Attachment #610140 - Flags: review?(dcamp)
Attachment #610140 - Flags: feedback?(dcamp)
(Assignee)

Updated

5 years ago
Attachment #610140 - Flags: review?(jonas)
Attachment #610140 - Flags: feedback?(jonas)
Comment on attachment 610140 [details] [diff] [review]
(Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation
[Checked in: Comment 8]

I'm sorry, but I don't know the safe-browsing code well enough to review this (I don't really know it at all in fact). Maybe someone from the network team knows this stuff?
Attachment #610140 - Flags: review?(jonas)
Attachment #610140 - Flags: feedback?(jonas)
Attachment #610140 - Flags: feedback?(jduell.mcbugs)
(Assignee)

Comment 6

5 years ago
(In reply to Jonas Sicking (:sicking) from comment #5)

> I'm sorry, but I don't know the safe-browsing code well enough to review
> this (I don't really know it at all in fact).

I thought you would know it, as it looks like you r+/sr+ bug 441359 which added this code :-|

> Maybe someone from the network team knows this stuff?

Isn't Dave Camp, who wrote this code, around anymore?
(Assignee)

Updated

5 years ago
Attachment #610140 - Flags: review?(benjamin)
Attachment #610140 - Flags: feedback?(benjamin)

Updated

5 years ago
Attachment #610140 - Flags: review?(dcamp) → review+

Comment 7

5 years ago
r+'ed - :gcp (now cc'ed) is a capable safebrowsing reviewer too.
Attachment #610140 - Flags: feedback?(jduell.mcbugs)
(Assignee)

Comment 8

5 years ago
Comment on attachment 610140 [details] [diff] [review]
(Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation
[Checked in: Comment 8]

https://hg.mozilla.org/mozilla-central/rev/2c8a0c3b47f4


Dave, what about comment 4?
Attachment #610140 - Attachment description: (Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation → (Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation [Checked in: Comment 8]
Attachment #610140 - Flags: review?(benjamin)
Attachment #610140 - Flags: feedback?(benjamin)
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox-esr10: --- → wontfix
status-firefox11: --- → wontfix
status-firefox12: --- → wontfix
status-firefox13: --- → wontfix
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1333486898.1333487878.6664.gz&fulltext=1
OS X 10.6 comm-central-trunk debug test mochitests-5/5 on 2012/04/03 14:01:38
{
9640 INFO TEST-PASS | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier.html | Should not load bad javascript - untouched should equal untouched
9641 INFO TEST-PASS | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier.html | Should not load bad css - visible should not equal hidden

9644 INFO TEST-PASS | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier_worker.html | blocked evilWorker.js - success should equal success
9645 INFO TEST-PASS | /tests/toolkit/components/url-classifier/tests/mochitest/test_classifier_worker.html | blocked importScripts('evilWorker.js') - success should equal success
}

V.Fixed
Status: RESOLVED → VERIFIED
(Assignee)

Updated

5 years ago
Attachment #610140 - Flags: feedback?(gpascutto)
Comment on attachment 610140 [details] [diff] [review]
(Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation
[Checked in: Comment 8]

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

Regarding the XXX, isn't this the answer? 
// Make sure the css did not load.
Attachment #610140 - Flags: feedback?(gpascutto) → feedback+
(Assignee)

Comment 11

5 years ago
(In reply to Gian-Carlo Pascutto (:gcp) from comment #10)
> Regarding the XXX, isn't this the answer? 
> // Make sure the css did not load.

No, that code is related to evil.css, not import.css.

Updated

5 years ago
Attachment #610140 - Flags: feedback?(dcamp)
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.