681920 - Failure in /testSecurity/testSafeBrowsingNotificationBar.js | Expression "{"value":"blocked-badware-page"}" returned null. Anonymous == false

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Alex Lakatos[:AlexLakatos]]

TEST: testSafeBrowsingNotificationBar.js::testNotificationBar
ERROR: Expression "{"value":"blocked-badware-page"}" returned null. Anonymous == false
WHEN: 2011-08-25
FIRST: 2011-08-25
FREQ: 9

Comment 1

[u415141]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/17462921

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Alex, please use the template we have created on the wiki page when you file a new test failure.

On which branches is that test failing? Also provide a link to the failure details list with a start time only and not a single report in the URL field. thanks.

Comment 3

[Alex Lakatos[:AlexLakatos]]

I've used the template from https://wiki.mozilla.org/QA/Mozmill_Test_Automation/Fixing_Broken_Tests#Filing_a_Bug . Is that deprecated in any way? Have I missed anything from there?

I've also updated the link to failure details. Sorry about that, I copied the wrong link.

The test is only failing on mozilla-beta as far as I can see.

Comment 4

[u279076]

(In reply to Alex Lakatos from comment #3)
> I've used the template from
> https://wiki.mozilla.org/QA/Mozmill_Test_Automation/
> Fixing_Broken_Tests#Filing_a_Bug . Is that deprecated in any way? Have I
> missed anything from there?

The template there was missing the BRANCH field -- I've added it.

Comment 5

[Alex Lakatos[:AlexLakatos]]

Attachment: skip patch

Patch for skipping the test while I'm debugging it, as per the 24h skip policy.

Comment 6

[u279076]

Comment on attachment 555757 [details] [diff] [review]
skip patch

>+// Bug 681920 - Failure in /testSecurity/testSafeBrowsingNotificationBar.js
>+// Expression "{"value":"blocked-badware-page"}" returned null. Anonymous == false
>+setupModule.__force_skip__ = "Bug 681920 - Failure in /testSecurity/testSafeBrowsingNotificationBar.js |" +
>+                             "Expression '{'value':'blocked-badware-page'}' returned null. Anonymous == false";
>+teardownModule.__force_skip__ = "Bug 681920 - Failure in /testSecurity/testSafeBrowsingNotificationBar.js |" +
>+                                "Expression '{'value':'blocked-badware-page'}' returned null. Anonymous == false";

A space should be added after the "|" -- I will do this prior to check-in.

Comment 7

[u279076]

Attachment: skip patch (all) [checked-in]

Comment 8

[u279076]

Comment on attachment 555798 [details] [diff] [review]
skip patch (all) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/5280734e7c4a (mozilla-beta)

Comment 9

[Remus Pop (:RemusPop)]

This failure happens from the .com to .org redirection.
The tab bar that should be visible with 2 buttons disappears after the redirection.

There is another way to avoid the redirection if that is not needed. We can set the pref network.http.redirection-limit to 0 and in this case the tab bar with the 2 buttons will remain visible. 

After the avoidance we can set the pref back to default (20) and continue the test normally.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Good idea. I don't have a problem with it if we reset the pref right after we were able to access the notification bar and not at the end of the test. Does that full fix this failure?

Comment 11

[Remus Pop (:RemusPop)]

I've searched for a workaround with no luck. 
We may have a chance in https://bugzilla.mozilla.org/show_bug.cgi?id=662213 and just modify the test urls.

Comment 12

[u279076]

(In reply to Remus Pop (:RemusPop) from comment #11)
> I've searched for a workaround with no luck. 
> We may have a chance in https://bugzilla.mozilla.org/show_bug.cgi?id=662213
> and just modify the test urls.

I'm not completely clear about the reasoning behind deprecating the attack/trap pages. This change will affect all of our safe browsing tests. I'm inclined to think that we should create our own copies of these pages (on mozqa.com).

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #12)
> I'm not completely clear about the reasoning behind deprecating the
> attack/trap pages. This change will affect all of our safe browsing tests.
> I'm inclined to think that we should create our own copies of these pages
> (on mozqa.com).

We should do. At the same time we have to find out how we can get marked those pages as fraudulent. Seems like we have to talk to Kev, so we can get the URLs included by default. For the pages themselves I would propose to use those from mozilla.org. Anthony, can you take care of it? We should get a bug filed in the infrastructure component.

Comment 14

[Remus Pop (:RemusPop)]

Attachment: skip patch (1.9.2) [checked-in]

This skips the test on 1.9.2 branch.

I think we can apply that patch (that skips the test) from beta to aurora and default too because this issue is also present to those.

Comment 15

[u279076]

Comment on attachment 561675 [details] [diff] [review]
skip patch (1.9.2) [checked-in]

We should include a teardownModule skip as well. FWIW, I don't see why the beta patch can't be applied to other branches as well. We should definitely skip the test on all branches given the prevalence of this failure. I will test the beta patch and if it passes I will land it everywhere.

Comment 16

[u279076]

Comment on attachment 561675 [details] [diff] [review]
skip patch (1.9.2) [checked-in]

Nevermind, I see there is no teardownModule in this test. r+

Comment 17

[u279076]

Comment on attachment 561675 [details] [diff] [review]
skip patch (1.9.2) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/fcb936fa0363 (mozilla-1.9.2)

Comment 18

[u279076]

Comment on attachment 555798 [details] [diff] [review]
skip patch (all) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/f035862253d6 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/582053c7ee32 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/5d6989178619 (mozilla-release)
http://hg.mozilla.org/qa/mozmill-tests/rev/c6bdf2411e6f (mozilla-2.0)

Test is skipped across all branches.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

For the time being we could use a similar technique as given by the following code to add our own entries for mozilla.org:

http://mxr.mozilla.org/mozilla-central/source/browser/components/safebrowsing/content/malware-warden.js

Comment 20

[Remus Pop (:RemusPop)]

I'm not really sure what we could use from there.

I've found something else. Adding an entry to the database which Firefox uses to check if a website contains malware. 
Here it is:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#474

Comment 21

[u279076]

Is this just a workaround until they resolve bug 662213? If so, I would rather this test remain disabled until then and work on a final solution then.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This has been fixed on mozilla-central. Simply change the URL from mozilla.com to mozilla.org in the test.

Comment 23

[u279076]

Remus, can you please implement the change in comment 22 and see if it resolves this issue? Thanks.

Comment 24

[Remus Pop (:RemusPop)]

Attachment: patch v1 (default) [checked-in]

This only works for the default channel for now.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Can we get any progress on this bug? Probably we should auto-forward a review request to Anthony, once it has been internally reviewed by you.

Comment 26

[u279076]

Just a quick note before check-in; you can simplify the commit message to:

"Bug 681920 - Failure in /testSecurity/testSafeBrowsingNotificationBar.js. r=vlad.maniac r=ashughes"

I will make this change on check-in.

Comment 27

[u279076]

Comment on attachment 574283 [details] [diff] [review]
patch v1 (default) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e3488952343d (default)

Please submit follow up patches for other branches.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We can't check-in this change on other branches. At least not now. We have to wait for the usual train ride, or for additional check-ins on bug 693389. 

For now we should close this bug, because there is nothing we can do.

Comment 29

[u279076]

Remus, please verify with the next recent testrun and update the spreadsheet.