[mozmill] Test for verifying GREY Larry



Mozilla QA
Mozmill Tests
8 years ago
7 years ago


(Reporter: ashughes, Assigned: ashughes)


Firefox Tracking Flags

(Not tracked)



(2 attachments, 1 obsolete attachment)



8 years ago
This is a placeholder bug for creating a Mozmill test script for https://litmus.mozilla.org/show_test.cgi?id=6777

Comment 1

8 years ago
Created attachment 395227 [details] [diff] [review]
Initial patch
Attachment #395227 - Flags: review?(hskupin)
Comment on attachment 395227 [details] [diff] [review]
Initial patch

>+// Include necessary modules
>+var RELATIVE_ROOT = '../../shared-modules';

We don't use any shared module in this test. You can remove it.

>+var testLarryGrey = function() {
>+  // Go to a "grey" website
>+  controller.open("http://www.mozilla.org");

Just a question from a non-native speaker. Which version is more common? Grey or Gray? Most times I read Gray. So I wonder.

>+  controller.waitForPageLoad(controller.tabs.activeTab);

For the future we don't wanna use the activeTab as param. Can you please remove it?

>+  // Check the favicon
>+  controller.assertProperty(new elementslib.ID(controller.window.document, "page-proxy-favicon"), 
>+                            "src" ,"http://www.mozilla.org/images/mozilla-16.png");

That one fails now since we have been updated the design of the website. You should add a further check that there is no label set in this case on the button.

>+  if (cssInfo.getPropertyValue('background-image') != 'url(chrome://browser/skin/urlbar/startcap.png)') {
>+    throw 'Identity Box is not shown with a grey background when it should be';

We should remove this. The className check of the identity-box is enough.

>+  // Check the identity box darkens
>+  var cssInfo = controller.window.getComputedStyle(controller.window.document.getElementById("identity-box"), "");  
>+  if (cssInfo.getPropertyValue('background-image') != 'url(chrome://browser/skin/urlbar/startcap-active.png)') {
>+    throw 'Identity Box is not shown with a darkened grey background when it should be';
>+  }

I don't think that we have to test this part. The elements which are shown are much more important. And this is an url check too.

>+  // Check that the Larry UI is unknown (aka Grey)
>+  controller.assertProperty(doorhanger, "className", "unknownIdentity");  

Just a note.. the doorhanger class name is updated once the identity box has been clicked and not immediately. So that way seems to be the right approach for this test instead of using the class name of the identity-box itself.

>+  // Check the More Information button
>+  var moreInfoButton = new elementslib.ID(controller.window.document, "identity-popup-more-info-button");
>+  controller.click(moreInfoButton);
>+  controller.sleep(2000);

Please reduce the time here. 500ms are enough.

>+  // Check the Page Info - Security Tab
>+  var pageInfoController = new mozmill.controller.MozMillController(mozmill.wm.getMostRecentWindow('Browser:page-info'));

Can you please split those lines into two separate ones? Just to make it more readable and that we follow the 80 chars per line as best as possible where we can.

>+  // Check that the Security tab is selected by default
>+  pageInfoController.assertProperty(new elementslib.ID(pageInfoController.window.document, "securityTab"), "selected", "true");

Can you please change it to assertEval and wait for this property?

>+  // XXX: I can't seem to find this string in any .properties or .dtd files
>+  var webIDOwnerLabel = new elementslib.ID(pageInfoController.window.document, "security-identity-owner-value");
>+  pageInfoController.assertValue(webIDOwnerLabel, "This web site does not supply ownership information.");

Can be done like this way:

>+  // Check the Verifier label for "Not Specified"
>+  // XXX: I can't seem to find this string in any .properties or .dtd files
>+  var webIDVerifierLabel = new elementslib.ID(pageInfoController.window.document, "security-identity-verifier-value");
>+  pageInfoController.assertValue(webIDVerifierLabel, "Not specified");

See above link.
Attachment #395227 - Flags: review?(hskupin) → review-

Comment 3

8 years ago
Created attachment 397110 [details] [diff] [review]
Attachment #395227 - Attachment is obsolete: true
Attachment #397110 - Flags: review?(hskupin)
Comment on attachment 397110 [details] [diff] [review]

Looks good so far. Only some small nits.

>+  // Check the favicon
>+  controller.assertProperty(new elementslib.ID(controller.window.document, "page-proxy-favicon"), 
>+                            "src" ,"http://www.mozilla.org/favicon.ico");

For a better readability it makes more sense to move the element creation to its own line. I changed this and also for other lines.

>+  // Get a reference to the Page Info Security Tab string bundle   
>+  var pageInfoBundle = new elementslib.ID(pageInfoController.window.document, "pageinfobundle");

We shouldn't rely on elements inside XUL documents. We have the UtilsAPI.getProperty function which we should use for any access to strings which are located in property files.

Whitespace issues were mostly fixed automatically by my editor.

I'll attach a diff so you know what has been slightly changed before the checkin.

Thanks Anthony.
Attachment #397110 - Flags: review?(hskupin) → review+
Created attachment 397824 [details] [diff] [review]
Diff to last patch
Landed as:
Last Resolved: 8 years ago
Resolution: --- → FIXED
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.
Component: Security → Mozmill Tests
Product: Firefox → Mozilla QA
QA Contact: firefox → mozmill-tests
You need to log in before you can comment on or make changes to this bug.