578580 - Add category utilities for browser tests

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Ben Parr [:bparr]]

Add category utilities to test/browser/head.js so that tests don't have to implement the same thing over and over again (e.g. opening a category).

Comment 1

[Ben Parr [:bparr]]

Attachment: Patch

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 457254 [details] [diff] [review]
Patch

This looks good. Will take another spin after the proposed changes.

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_bug572561.js b/toolkit/mozapps/extensions/test/browser/browser_bug572561.js

> // Test that restarting the add-on manager with a non-active install
> // does not cause the locale category to show
> add_test(function() {
>   restart_manager(gManagerWindow, null, function(aWindow) {
>     gManagerWindow = aWindow;
>+    gCategoryUtilities = new CategoryUtilities(gManagerWindow);

You don't seem to use this after this point so probably no point in creating it.

>     check_hidden(true);
>     run_next_test();
>   });
> });
> 
> // Test that installing the install shows the locale category
> add_test(function() {
>   gInstall.install();
> });
> 
> // Test that restarting the add-on manager does not cause the locale category
> // to become hidden
> add_test(function() {
>   restart_manager(gManagerWindow, null, function(aWindow) {
>     gManagerWindow = aWindow;
>+    gCategoryUtilities = new CategoryUtilities(gManagerWindow);

Ditto

>diff --git a/toolkit/mozapps/extensions/test/browser/head.js b/toolkit/mozapps/extensions/test/browser/head.js

>+CategoryUtilities.prototype = {
>+  window: null,
>+
>+  get currentCategory() {
>+    isnot(this.window, null, "Should not get current when manager window is not loaded");
>+    var currentViewId = this.window.gViewController.currentViewId;
>+    var view = this.window.gViewController.parseViewId(currentViewId);
>+    return (view.type == "list") ? view.param : view.type;
>+  },

I think I would prefer this to be called selectedCategory and to get it from the selected richlistitem. These are UI tests after all so it's more important that the UI state is correct than the internal state.

>+
>+  getViewId: function(aCategoryType) {
>+    var LIST_CATEGORIES = ["locale", "searchengine", "extension" , "theme", "plugin"];
>+    if (LIST_CATEGORIES.indexOf(aCategoryType) >= 0)
>+      return "addons://list/" + aCategoryType;

To avoid the hardcoded list here you could just search the document for elements with attribute value="addons://list/<aCategoryType>". That would save us altering this if we change anything.

>+
>+    return "addons://" + aCategoryType + "/";
>+  },
>+
>+  get: function(aCategoryType) {
>+    isnot(this.window, null, "Should not get category when manager window is not loaded");
>+    return this.window.gCategories.get(this.getViewId(aCategoryType));
>+  },
>+
>+  isShowing: function(aCategory) {

Name this isVisible.

>+    isnot(this.window, null, "Should not check showing state when manager window is not loaded");
>+    if (aCategory.hasAttribute("disabled") &&
>+        aCategory.getAttribute("disabled") == "true")
>+      return false;
>+
>+    var style = this.window.document.defaultView.getComputedStyle(aCategory, "");
>+    return style.display != "none" && style.visibility == "visible";
>+  },
>+
>+  isTypeShowing: function(aCategoryType) {
>+    return this.isShowing(this.get(aCategoryType));
>+  },

isTypeVisible.

>+
>+  open: function(aCategory, aCallback) {
>+    isnot(this.window, null, "Should not open category when manager window is not loaded");
>+    ok(this.isShowing(aCategory), "Category should be showing if attempting to open it");
>+
>+    EventUtils.synthesizeMouse(aCategory, 2, 2, { }, this.window);
>+
>+    if (aCallback)
>+      wait_for_view_load(this.window, aCallback);
>+  },
>+
>+  openType: function(aCategoryType, aCallback) {
>+    this.open(this.get(aCategoryType), aCallback);
>+  },

Comment 3

[Ben Parr [:bparr]]

Attachment: Patch v2

Kept the "gCategoryUtilities = new CategoryUtilities(gManagerWindow);" lines in browser_bug572561.js because gCategoryUtilities is used by the check_hidden() function. Besides that, I made the rest of Dave's changes.

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 457448 [details] [diff] [review]
Patch v2

>+  get: function(aCategoryType) {
>+    isnot(this.window, null, "Should not get category when manager window is not loaded");
>+    var viewId = "addons://list/" + aCategoryType;
>+    var category = this.window.gCategories.get(viewId);
>+    if (category)
>+      return category;
>+
>+    viewId = "addons://" + aCategoryType + "/";
>+    var category = this.window.gCategories.get(viewId);
>+    if (category)
>+      return category;

I want to avoid using the add-ons manager objects where possible and focus on the raw DOM. You should be able to get the richlist for the categories and use list.getElementsByAttribute("value", viewId) to get the category here.

Comment 5

[Ben Parr [:bparr]]

Attachment: Patch v3

Made Dave's changes.

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 457646 [details] [diff] [review]
Patch v3

Cool, thanks

Comment 7

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/5fd0b1867f79

Comment 8

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

Marking as verified fixed based on its landing and green test results.