Timeout exceeded for waitForElement ID: getMeOutButton in testSafeBrowsingWarningPages.js

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: vladmaniac)

Tracking

({regression})

unspecified
regression
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(firefox15 fixed, firefox16 fixed, firefox17 fixed, firefox18 fixed, firefox-esr10 fixed)

Details

(Whiteboard: [mozmill-test-failure] s=q3 u=failure c=security p=1)

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Test: testWarningPages
Module: testSafeBrowsingWarningPages.js
Error: Timeout exceeded for waitForElement ID: getMeOutButton
Branch: 11.0a1 (Nightly)
Platform: Windows NT 5.0.2195, Windows NT 5.1.2600, Windows NT 6.0.6002, Windows NT 6.1.7601,  Mac OS X 10.6.8, Linux Ubuntu 10.10

http://mozmill-release.brasstacks.mozilla.com/#/functional/failure?test=%2FtestSecurity%2FtestSafeBrowsingWarningPages.js&func=testSafeBrowsingWarningPages.js%3A%3AtestWarningPages

Failing since November 11th, 2011.
This could be a regression in Firefox 11. Aaron, can you check to see if there were any recent changes?

FYI, we have had this issue before: bug 655885
(Reporter)

Comment 2

6 years ago
activities in bug 693389 looks to be the culprit from the associated push log
Looks to be. We have been dealing with this in one-off tests. I suggest filing a bug to do a broad sweep to update all tests referencing mozilla.com safe-browsing pages.

Aaron, would you be willing to take care of that?
(Reporter)

Comment 4

6 years ago
Created attachment 576836 [details] [diff] [review]
default (mozilla.com to mozilla.org)

All other tests under the default branch appear to be good with those recent changes. Note I have not tested this.
Test still fails for me, even with this change. Aaron, could you please continue to investigate? In the meantime, please submit a patch which marks this test skipped.

Thanks
Assignee: nobody → aaron.train
Status: NEW → ASSIGNED
(Reporter)

Comment 6

6 years ago
Created attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]
Attachment #577448 - Flags: review?
(Reporter)

Updated

6 years ago
Attachment #577448 - Flags: review? → review?(anthony.s.hughes)
Attachment #577448 - Flags: review?(anthony.s.hughes) → review+
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/1194b3ad8f19 (default)
Attachment #577448 - Attachment description: patch - disable testSafeBrowsingWarningPages (default) → disable test (default) [checked-in]
Test has now been skipped. Aaron, do you have time to investigate this failure? If not I will have to re-assign it.

Thanks
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mozmill-test-skipped]
Assignee: aaron.train → nobody
Status: ASSIGNED → NEW
This test remains enabled on release branch and I noticed it's failing there; porting disable patch to release branch.
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/7b81f660a18f (mozilla-release)
Attachment #577448 - Attachment description: disable test (default) [checked-in] → disable test (default) [landed:default,release]
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Noticed a failure today on ESR branch so I've ported the disable patch.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e8cfa158b5f0 (mozilla-esr10)
Attachment #577448 - Attachment description: disable test (default) [landed:default,release] → disable test [landed:default,release,esr]
I noticed this is the only bug with [mozmill-test-skipped] that s not currently assigned. Can someone pick this up?
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure][mozmill-test-skipped] s=2012-8-27 u=failure c=security p=1
(Assignee)

Updated

5 years ago
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
(Assignee)

Comment 13

5 years ago
This is failing because bug 468313 has landed. 
So this makes this test invalid at least for the 'http://www.mozilla.com/firefox/its-an-attack.html'
Do you suggest we split 'testSafeBrowsingWarningPages.js' into two tests? One to handle 'its a trap' page and one to handle the 'its an attack' because for the last one we need different checks. 

In any case, this is not about enabling this test anymore, we need to consult Anthony to get some feedback from him before we proceed because a manual test case is not present in MozTrap at the moment afaik.
Please see bug 468313 comment 35 for the solution.
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #13)
> Do you suggest we split 'testSafeBrowsingWarningPages.js' into two tests?
> One to handle 'its a trap' page and one to handle the 'its an attack'
> because for the last one we need different checks. 

You can file a follow-up bug for that but it doesn't affect our work to get this failure fixed.
Blocks: 468313
Keywords: regression
(Assignee)

Comment 16

5 years ago
Created attachment 660014 [details] [diff] [review]
proposed fix v1.0

Then its easy. We are setting the permission target wrongly because mozilla.com redirects to mozilla.org. A simple domain change in the test fixes the problem
Attachment #660014 - Flags: review?(dave.hunt)
(Assignee)

Updated

5 years ago
Attachment #660014 - Flags: review?(hskupin)
Comment on attachment 660014 [details] [diff] [review]
proposed fix v1.0

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

You missed to enable the test in the manifest. Things like that should really be caught by your internal review.

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +18,5 @@
>  }
>  
>  function teardownModule(module) {
>    // Clear the Safe Browsing permission
> +  utils.removePermission("www.mozilla.org", "safe-browsing");

Please move the domain out to a global constant so that we do not have to fix that on multiple places when something gets changed.
Attachment #660014 - Flags: review?(hskupin)
Attachment #660014 - Flags: review?(dave.hunt)
Attachment #660014 - Flags: review-
(Assignee)

Comment 18

5 years ago
Created attachment 660027 [details] [diff] [review]
fix patch v1.1

* fixed
Attachment #660014 - Attachment is obsolete: true
Attachment #660027 - Flags: review?(hskupin)
(Assignee)

Updated

5 years ago
Attachment #660027 - Flags: review?(dave.hunt)
Comment on attachment 660027 [details] [diff] [review]
fix patch v1.1

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

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +25,5 @@
>  }
>  
>  var testWarningPages = function() {
> +  var urls = ['http://www.mozilla.org/firefox/its-a-trap.html',
> +              'http://www.mozilla.org/firefox/its-an-attack.html'];

Please do internal reviews before uploading updated patches, Vlad! I don't think that this wouldn't have caused a discussion. The domain name is still present here.
Attachment #660027 - Flags: review?(hskupin)
Attachment #660027 - Flags: review?(dave.hunt)
Attachment #660027 - Flags: review-
(Assignee)

Comment 20

5 years ago
Created attachment 660029 [details] [diff] [review]
patch v1.2

this passed internal review
Attachment #660027 - Attachment is obsolete: true
Attachment #660029 - Flags: review?(hskupin)
(Assignee)

Updated

5 years ago
Attachment #660029 - Flags: review?(dave.hunt)
Comment on attachment 660029 [details] [diff] [review]
patch v1.2

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

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +25,5 @@
>  }
>  
>  var testWarningPages = function() {
> +  var urls = ['http://' + DOMAIN_NAME + '/firefox/its-a-trap.html',
> +              'http://' + DOMAIN_NAME + '/firefox/its-an-attack.html'];

A global constant for those test pages would be nice. Mind to file a mentored follow-up bug?
Attachment #660029 - Flags: review?(hskupin)
Attachment #660029 - Flags: review?(dave.hunt)
Attachment #660029 - Flags: review+
Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/eac1daad96b4

Please check the results tomorrow so that we can land this patch on the other branches.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox-esr10: --- → affected
status-firefox15: --- → disabled
status-firefox16: --- → disabled
status-firefox17: --- → disabled
status-firefox18: --- → fixed
Resolution: --- → FIXED
(Reporter)

Comment 23

5 years ago
Glad to see a fix here :-)
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=2012-8-27 u=failure c=security p=1 → [mozmill-test-failure][mozmill-test-skipped] s=q3 u=failure c=security p=1
(Assignee)

Comment 24

5 years ago
(In reply to Henrik Skupin (:whimboo) from comment #22)
> Landed on default:
> http://hg.mozilla.org/qa/mozmill-tests/rev/eac1daad96b4
> 
> Please check the results tomorrow so that we can land this patch on the
> other branches.

I see no failures today so I think we can land this on other branches
(Assignee)

Comment 25

5 years ago
Added bug 790597 for the follow up to move the url variable in the global scope as a constant. 
Henrik, mind adding the necessary whiteboard entries? I would add them but not sure I can get it right
http://hg.mozilla.org/qa/mozmill-tests/rev/600510f8a7f2 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/9daf3a935e1c (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/ef42f41a046a (release)

Patch cannot be applied on esr10 due to differences in manifest.ini. Please come up with a working backport patch. In the future please check that before you give the go for backporting.
status-firefox15: disabled → fixed
status-firefox16: disabled → fixed
status-firefox17: disabled → fixed
(Assignee)

Comment 27

5 years ago
Created attachment 660729 [details] [diff] [review]
esr patch v1.0

this is the backport patch for esr, should apply and work as expected
Attachment #660729 - Flags: review?(hskupin)
(Assignee)

Updated

5 years ago
Attachment #660729 - Flags: review?(dave.hunt)
Attachment #660729 - Flags: review?(hskupin)
Attachment #660729 - Flags: review?(dave.hunt)
Attachment #660729 - Flags: review+
Landed on esr10 as:
http://hg.mozilla.org/qa/mozmill-tests/rev/867264df02f7

Vlad, please update the Litmus test on the Firefox 10 branch.
status-firefox-esr10: affected → fixed
Flags: in-litmus?(vlad.mozbugs)
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=q3 u=failure c=security p=1 → [mozmill-test-failure] s=q3 u=failure c=security p=1
You need to log in before you can comment on or make changes to this bug.