Last Comment Bug 739687 - [SeaMonkey] test_classifier.html and test_classifier_worker.html fail
: [SeaMonkey] test_classifier.html and test_classifier_worker.html fail
Status: VERIFIED FIXED
[perma-orange]
:
Product: Toolkit
Classification: Components
Component: Safe Browsing (show other bugs)
: Trunk
: All All
: P2 major (vote)
: Firefox 14
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on:
Blocks: SmTestFail 441359 738884
  Show dependency treegraph
 
Reported: 2012-03-27 10:23 PDT by Serge Gautherie (:sgautherie)
Modified: 2014-05-27 12:25 PDT (History)
6 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
wontfix
wontfix


Attachments
(Av1) test_classifier.html and test_classifier_worker.html: Set preference they depend on, Some rewrite and documentation [Checked in: Comment 8] (8.98 KB, patch)
2012-03-28 08:03 PDT, Serge Gautherie (:sgautherie)
dcamp: review+
gpascutto: feedback+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2012-03-27 10:23:16 PDT
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 Philip Chee 2012-03-27 13:17:33 PDT
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 Philip Chee 2012-03-27 21:13:02 PDT
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/)
Comment 3 Serge Gautherie (:sgautherie) 2012-03-28 07:42:09 PDT
(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.
Comment 4 Serge Gautherie (:sgautherie) 2012-03-28 08:03:11 PDT
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 ?
Comment 5 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-04-02 13:19:22 PDT
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?
Comment 6 Serge Gautherie (:sgautherie) 2012-04-02 13:43:13 PDT
(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?
Comment 7 Dave Camp (:dcamp) 2012-04-02 16:45:15 PDT
r+'ed - :gcp (now cc'ed) is a capable safebrowsing reviewer too.
Comment 8 Serge Gautherie (:sgautherie) 2012-04-03 07:18:15 PDT
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?
Comment 9 Serge Gautherie (:sgautherie) 2012-04-03 17:23:30 PDT
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
Comment 10 Gian-Carlo Pascutto [:gcp] 2012-04-04 03:47:47 PDT
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.
Comment 11 Serge Gautherie (:sgautherie) 2012-04-04 03:54:15 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.