503984 - [mozmill] - Test the safebrowsing drop-down bar

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Aakash Desai [:aakashd]]

Attachment: testSafeBrowsing/testSafeBrowsingPopDownBar.js

This test case ignores the attack site and web forgery warnings pages and tests the functionality of the get-me-out-of-here and this-is-not-a... buttons on the popdown bar.

It depends on the resolution of bug 492426 as the testscript cannot click on the second instance of the warning bar since mozmill seems to touch the action rather than "physically" push the click on the popdown bar.

litmus test case represented:
 * Testcase ID #7804 - Test the safebrowsing drop-down bar

Comment 1

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

Comment on attachment 388361 [details]
testSafeBrowsing/testSafeBrowsingPopDownBar.js

> testSafeBrowsing/testSafeBrowsingPopDownBar.js

The bar you will see here is called notification bar. Can you also update the Litmus test? Further you can also remove the SafeBrowsing part from the file name since we already have it in the folder name.

> * The Original Code is Mozilla Mozmill Test Code.
> *
> * The Initial Developer of the Original Code is Mozilla Corporation.
> * Portions created by the Initial Developer are Copyright (C) 2009
> * the Initial Developer. All Rights Reserved.
> *
> * Contributor(s):
> *   Aakash Desai <[e-mail]>
> *
> * Alternatively, the contents of this file may be used under the terms of
> * either the GNU General Public License Vertributed under the License is distributed on an "AS IS" basis,
> * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
> * for the specific language governing rights and limitations under the
> * License.
> *
> * The Original Code is Mozilla Mozmill Test Code.
> *
> * The Initial Developer of the Original Code is Mozihers to
> * use your version of this file under the terms of the MPL, indicate your
> * decision by deleting the provisions above and replace them with the notice
> * and other provisions required by the GPL or the LGPL. If you do not delete
> * the provisions above, a recipient may use your version of this file under
> * the terms of any one of the MPL, the GPL or the LGPL.
> *
> * ***** END LICENSE BLOCK ***** */

Something messed up in the license block. Please also remove "Mozill" from the Original Code line.

>  // Go to Mozilla's safebrowsing test pages to bring up the warning page
>  var badSites = ['http://www.mozilla.com/firefox/its-a-trap.html','http://www.mozilla.com/firefox/its-an-attack.html'];

Can you please update this accordingly to the comments on the other safe browsing test? Also with the page type information.

>    // Go to one of the mozilla phishing protection test page
>    controller.open(badSites[i]);
>    controller.sleep(750);

Please use waitforPageLoad with a timeout value of ~1000 ms.

>    // Waits for the ignoreWarning button to be loaded onto the page and then clicks on the button
>    checkThenClickIgnoreButton();
>    checkVerificationButton();

There is no need to having it split up into two separate functions. checkIgnoreButton should be fine.

>    // Go back to the warning page and then the phishing site
>    controller.open(badSites[i]);

Why you don't use goBack()? That's faster.

>    checkThenClickIgnoreButton();

And why it is called again? Shouldn't it be the "get me out of here" button?

>var checkThenClickIgnoreButton = function() {
>  controller.waitForElement(ignoreButton);	
>	
>  // Go to the phishing site and test the popdown bar's functionality
>  controller.click(ignoreButton);

controller.waitThenClick is preferred here.

>var checkVerificationButton = function(badPage) {

Please use an identifier as parameter instead. That will give us a change to not have to check an URL to differentiate between the type of forgery page.

>    // Wait for the web forgery report button to be loaded and click on it
>    var notWebForgeryButton = new elementslib.Lookup(controller.window.document, "/id('main-window')/id('browser')/id('appcontent')/id('content')/anon({'anonid':'tabbox'})/anon({'anonid':'panelcontainer'})/[2]/{'label':'Reported Web Forgery!','value':'blocked-badware-page','image':'chrome://global/skin/icons/blacklist_favicon.png','priority':'9','type':'critical'}/{'label':'This isn't a web forgeryâ¦'}");

That will not work. First, there are still labels in that Lookup string. Further please check the popup blocker test which has a similar test of the notification bar. You can use it to check for the correct lookup string.

>    controller.waitForElement(notWebForgeryButton);
>    controller.click(notWebForgeryButton);

Please use waitThenClick too.

>	// Verify not-a-web-forgery report page is loaded
>    controller.waitForElement(new elementslib.Link(controller.tabs.activeTab, "Badware Clearinghouse"));
>    controller.assertNode(new elementslib.Link(controller.tabs.activeTab, "Mozilla"));

Wouldn't it be better to check for the correct url? I cannot load the page because it times out. Is there a redirect? Further it is opened in a new tab. You should close it before processing further.

>    // Wait for the attack site report button to be loaded and click on it
>    var notAttackSiteButton = new elementslib.Lookup(controller.window.document, "/id('main-window')/id('browser')/id('appcontent')/id('content')/anon({'anonid':'tabbox'})/anon({'anonid':'panelcontainer'})/[0]/{'label':'Reported Attack Site!','value':'blocked-badware-page','image':'chrome://global/skin/icons/blacklist_favicon.png','priority':'9','type':'critical'}/{'label':'This isn't an attack siteâ¦'}");

See comment above.

>	// Verify not-an-attack-site report page is loaded	
>    var urlField = new elementslib.ID(controller.tabs.activeTab, "url");

What element are you trying to access here? Nothing can be found and the test is marked as failed.

>var testGetMeOutButton = function(badPage) {

Please use a type parameter here too and don't give it a name with test as prefix. In such a case it will be handled as a separate test.

>  	// Wait for and test the get me out of here button
>    var getMeOutAttackButton = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})/[0]/{"label":"Reported Attack Site!","value":"blocked-badware-page","image":"chrome://global/skin/icons/blacklist_favicon.png","priority":"9","type":"critical"}/{"label":"Get me out of here!","accesskey":"G"}');

See above comment.

>  	// Wait for and test the get me out of here button
>    var getMeOutForgeryButton = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})/[0]/{"label":"Reported Web Forgery!","value":"blocked-badware-page","image":"chrome://global/skin/icons/blacklist_favicon.png","priority":"9","type":"critical"}/{"label":"Get me out of here!","accesskey":"G"}');	

Here too.

>  if ( locationBar.getNode().value.indexOf('mozilla') == -1) {
>    throw "Failed: getMeOutButton send user to a non-Mozilla related website.";

For the default home page check please see my comment in the other test.

Further I miss the test for the x button. Because you have to use the notification bar again you should find a reasonable template for the elementlib lookup method. So you only have to add the string for the specific element.

Also please check the whitespaces too.

Comment 2

[Aakash Desai [:aakashd]]

>    // Waits for the ignoreWarning button to be loaded onto the page and then clicks on the button
>    checkThenClickIgnoreButton();
>    checkVerificationButton();

checkThenClickIgnoreButton() is used twice and is a separate action than the checkVerificationButton() and checkGetMeOutOfHereButtion(). This is easier to read and much more functional because it separates different actions and leaves each function with one general thing to do.

>    // Go back to the warning page and then the phishing site
>    controller.open(badSites[i]);


controller.goBack() doesn't work for whatever reason. controller.open() one works perfectly fine.

>	// Verify not-a-web-forgery report page is loaded
>    controller.waitForElement(new elementslib.Link(controller.tabs.activeTab, "Badware Clearinghouse"));
>    controller.assertNode(new elementslib.Link(controller.tabs.activeTab, "Mozilla"));

There's a redirect and it can't be created via concatenating strings, so I'm forced to use elements. It may be due to the language used in the links. I'll change "Badware Clearinghouse" to something more static.


>	// Verify not-an-attack-site report page is loaded	
>    var urlField = new elementslib.ID(controller.tabs.activeTab, "url");

This is the element.Id of the "url" field in the submission form at the bottom of the page.


I will look into the popUpBlocked implementation with ashughes. As for the other vets, they've been made. I'll upload the testscript after making the notification bar changes.

Comment 3

[Aakash Desai [:aakashd]]

Attachment: testSafeBrowsing/testNotificationBar.js

Changes have been made and instead of looking up the label, I'm using the accesskey on the notification bar to verify their functionality.

Comment 4

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

Comment on attachment 390927 [details] [diff] [review]
testSafeBrowsing/testNotificationBar.js

That will not work because of different reasons. I'll attach a diff to your patch and explain it in detail. Check-in will happen soon.

Comment 5

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

Attachment: Follow-up patch

With the follow-up patch following parts have been fixed:

* Made the test pass for localized builds (as already said several times please never use labels and accesskeys because they differ for different locales)
* Even when we cancel the page load after 1s we should use waitForPageLoad instead of sleep
* Removed un-necessary badURL if/else cases due to Lookup simplifications
* When http://www.stopbadware.org is loaded we really should check the domain name in the location bar and not links on this page
* When the redirection to the default home page has been finished we should assert against the default home page

* Updated comments to jsdoc style
* Fixed license plate
* Fixed a lot of indentation and white-space nits (mostly done automatically by my editor Komodo)

Comment 6

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

Checked in as http://hg.mozilla.org/qa/mozmill-tests/rev/a68d70516b16

Comment 7

[Aakash Desai [:aakashd]]

looks good and I get what you're doing there. Thanks Henrik!

Comment 8

[Aakash Desai [:aakashd]]

verified on an hg pull and run

Comment 9

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

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.