Bug 1576735 - Canceling the create login form when no logins are in the list should revert to the previous view. r=MattN
authorJared Wein <jwein@mozilla.com>
Thu, 29 Aug 2019 15:44:25 +0000
changeset 551165 0bd14a48e5b8ba743292f475cc597d27783d30d5
parent 551164 d01e6a3eea5ca1c9e91ca4440a2edc35804f481f
child 551166 0bff78d1b4663da55d1499529c40916387c54d10
push id11865
push userbtara@mozilla.com
push dateMon, 02 Sep 2019 08:54:37 +0000
treeherdermozilla-beta@37f59c4671b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1576735
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1576735 - Canceling the create login form when no logins are in the list should revert to the previous view. r=MattN If there are no stored logins, the Cancel button is hidden. If there are visible stored logins, clicking the Cancel button will exit and move to the first login in the list. If there are no visible stored logins, clicking the Cancel button will reset the search filter, exit, and move to the first login in the list. This patch also fixes a race condition where we could end up with multiple observer notifications if about:logins was loaded multiple times very quickly. Differential Revision: https://phabricator.services.mozilla.com/D43544
browser/components/aboutlogins/AboutLoginsParent.jsm
browser/components/aboutlogins/content/aboutLogins.js
browser/components/aboutlogins/content/components/login-filter.js
browser/components/aboutlogins/content/components/login-item.css
browser/components/aboutlogins/content/components/login-list.js
browser/components/aboutlogins/tests/browser/browser_createLogin.js
testing/mochitest/BrowserTestUtils/ContentTaskUtils.jsm
--- a/browser/components/aboutlogins/AboutLoginsParent.jsm
+++ b/browser/components/aboutlogins/AboutLoginsParent.jsm
@@ -62,16 +62,17 @@ const augmentVanillaLoginObject = login 
   return Object.assign({}, login, {
     title,
   });
 };
 
 var AboutLoginsParent = {
   _l10n: null,
   _subscribers: new WeakSet(),
+  _observersAdded: false,
 
   // Listeners are added in BrowserGlue.jsm
   async receiveMessage(message) {
     // Only respond to messages sent from a privlegedabout process. Ideally
     // we would also check the contentPrincipal.originNoSuffix but this
     // check has been removed due to bug 1576722.
     if (message.target.remoteType != EXPECTED_ABOUTLOGINS_REMOTE_TYPE) {
       throw new Error(
@@ -242,23 +243,22 @@ var AboutLoginsParent = {
 
         messageManager.sendAsyncMessage(
           "AboutLogins:MasterPasswordResponse",
           token.isLoggedIn()
         );
         break;
       }
       case "AboutLogins:Subscribe": {
-        if (
-          !ChromeUtils.nondeterministicGetWeakSetKeys(this._subscribers).length
-        ) {
+        if (!this._observersAdded) {
           Services.obs.addObserver(this, "passwordmgr-crypto-login");
           Services.obs.addObserver(this, "passwordmgr-crypto-loginCanceled");
           Services.obs.addObserver(this, "passwordmgr-storage-changed");
           Services.obs.addObserver(this, UIState.ON_UPDATE);
+          this._observersAdded = true;
         }
         this._subscribers.add(message.target);
 
         let messageManager = message.target.messageManager;
 
         const logins = await this.getAllLogins();
         try {
           messageManager.sendAsyncMessage("AboutLogins:AllLogins", logins);
@@ -467,16 +467,17 @@ var AboutLoginsParent = {
   },
 
   async observe(subject, topic, type) {
     if (!ChromeUtils.nondeterministicGetWeakSetKeys(this._subscribers).length) {
       Services.obs.removeObserver(this, "passwordmgr-crypto-login");
       Services.obs.removeObserver(this, "passwordmgr-crypto-loginCanceled");
       Services.obs.removeObserver(this, "passwordmgr-storage-changed");
       Services.obs.removeObserver(this, UIState.ON_UPDATE);
+      this._observersAdded = false;
       return;
     }
 
     if (topic == "passwordmgr-crypto-login") {
       this.removeNotifications(MASTER_PASSWORD_NOTIFICATION_ID);
       this.messageSubscribers(
         "AboutLogins:AllLogins",
         await this.getAllLogins()
--- a/browser/components/aboutlogins/content/aboutLogins.js
+++ b/browser/components/aboutlogins/content/aboutLogins.js
@@ -23,16 +23,17 @@ if (searchParams.get("filter")) {
   gElements.loginFilter.value = searchParams.get("filter");
 }
 
 gElements.loginFilter.focus();
 
 function updateNoLogins() {
   document.documentElement.classList.toggle("no-logins", numberOfLogins == 0);
   gElements.loginList.classList.toggle("no-logins", numberOfLogins == 0);
+  gElements.loginItem.classList.toggle("no-logins", numberOfLogins == 0);
 }
 
 window.addEventListener("AboutLoginsChromeToContent", event => {
   switch (event.detail.messageType) {
     case "AllLogins": {
       gElements.loginList.setLogins(event.detail.value);
       numberOfLogins = event.detail.value.length;
       updateNoLogins();
--- a/browser/components/aboutlogins/content/components/login-filter.js
+++ b/browser/components/aboutlogins/content/components/login-filter.js
@@ -13,24 +13,31 @@ export default class LoginFilter extends
     let loginFilterTemplate = document.querySelector("#login-filter-template");
     let shadowRoot = this.attachShadow({ mode: "open" });
     document.l10n.connectRoot(shadowRoot);
     shadowRoot.appendChild(loginFilterTemplate.content.cloneNode(true));
 
     this._input = this.shadowRoot.querySelector("input");
 
     this.addEventListener("input", this);
+    window.addEventListener("AboutLoginsFilterLogins", this);
   }
 
   focus() {
     this._input.focus();
   }
 
   handleEvent(event) {
     switch (event.type) {
+      case "AboutLoginsFilterLogins": {
+        if (this.value != event.detail) {
+          this.value = event.detail;
+        }
+        break;
+      }
       case "input": {
         this._dispatchFilterEvent(event.originalTarget.value);
         break;
       }
     }
   }
 
   get value() {
--- a/browser/components/aboutlogins/content/components/login-item.css
+++ b/browser/components/aboutlogins/content/components/login-item.css
@@ -15,16 +15,17 @@
 
 form {
   flex-grow: 1;
 }
 
 :host([data-editing]) .edit-button,
 :host([data-editing]) .copy-button,
 :host([data-editing]) login-footer,
+:host([data-is-new-login].no-logins) .cancel-button,
 :host([data-is-new-login]) .delete-button,
 :host([data-is-new-login]) .origin-saved-value,
 :host([data-is-new-login]) .meta-info,
 :host([data-is-new-login]) login-footer,
 :host([data-is-new-login]) .login-item-title,
 :host(:not([data-is-new-login])) .new-login-title,
 :host(:not([data-editing])) .form-actions-row {
   display: none;
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -63,17 +63,17 @@ export default class LoginList extends H
     window.addEventListener("AboutLoginsInitialLoginSelected", this);
     window.addEventListener("AboutLoginsLoginSelected", this);
     window.addEventListener("AboutLoginsShowBlankLogin", this);
     this._list.addEventListener("click", this);
     this.addEventListener("keydown", this);
     this._createLoginButton.addEventListener("click", this);
   }
 
-  async render() {
+  render() {
     let visibleLoginGuids = this._applyFilter();
     this._updateVisibleLoginCount(visibleLoginGuids.size);
     this.classList.toggle("empty-search", visibleLoginGuids.size == 0);
 
     // Add all of the logins that are not in the DOM yet.
     let fragment = document.createDocumentFragment();
     for (let guid of this._loginGuidsSortedOrder) {
       if (this._logins[guid].listItem) {
@@ -154,29 +154,44 @@ export default class LoginList extends H
         this._applySort();
         this.render();
         break;
       }
       case "AboutLoginsClearSelection": {
         if (!this._loginGuidsSortedOrder.length) {
           return;
         }
-        // Select the first visible login after any possible filter is applied.
+
         let firstVisibleListItem = this._list.querySelector(
           ".login-list-item[data-guid]:not([hidden])"
         );
+        let newlySelectedLogin;
         if (firstVisibleListItem) {
-          let { login } = this._logins[firstVisibleListItem.dataset.guid];
+          newlySelectedLogin = this._logins[firstVisibleListItem.dataset.guid]
+            .login;
+        } else {
+          // Clear the filter if all items have been filtered out.
+          this.classList.remove("create-login-selected");
+          this._createLoginButton.disabled = false;
           window.dispatchEvent(
-            new CustomEvent("AboutLoginsLoginSelected", {
-              detail: login,
-              cancelable: true,
+            new CustomEvent("AboutLoginsFilterLogins", {
+              detail: "",
             })
           );
+          newlySelectedLogin = this._logins[this._loginGuidsSortedOrder[0]]
+            .login;
         }
+
+        // Select the first visible login after any possible filter is applied.
+        window.dispatchEvent(
+          new CustomEvent("AboutLoginsLoginSelected", {
+            detail: newlySelectedLogin,
+            cancelable: true,
+          })
+        );
         break;
       }
       case "AboutLoginsFilterLogins": {
         this._filter = event.detail.toLocaleLowerCase();
         this.render();
         break;
       }
       case "AboutLoginsInitialLoginSelected":
--- a/browser/components/aboutlogins/tests/browser/browser_createLogin.js
+++ b/browser/components/aboutlogins/tests/browser/browser_createLogin.js
@@ -20,16 +20,21 @@ add_task(async function test_create_logi
     ok(
       content.document.documentElement.classList.contains("no-logins"),
       "Should initially be in no logins view"
     );
     ok(
       loginList.classList.contains("no-logins"),
       "login-list should initially be in no logins view"
     );
+    is(
+      loginList._loginGuidsSortedOrder.length,
+      0,
+      "login list should be empty"
+    );
   });
 
   let testCases = [
     ["ftp://ftp.example.com/", "ftp://ftp.example.com"],
     ["https://example.com/foo", "https://example.com"],
     ["http://example.com/", "http://example.com"],
     [
       "https://testuser1:testpass1@bugzilla.mozilla.org/show_bug.cgi?id=1556934",
@@ -41,219 +46,330 @@ add_task(async function test_create_logi
   for (let i = 0; i < testCases.length; i++) {
     let originTuple = testCases[i];
     info("Testcase " + i);
     let storageChangedPromised = TestUtils.topicObserved(
       "passwordmgr-storage-changed",
       (_, data) => data == "addLogin"
     );
 
-    await ContentTask.spawn(browser, originTuple, async aOriginTuple => {
-      let loginList = Cu.waiveXrays(
-        content.document.querySelector("login-list")
-      );
-      let createButton = loginList._createLoginButton;
-      is(
-        content.getComputedStyle(loginList._blankLoginListItem).display,
-        "none",
-        "the blank login list item should be hidden initially"
-      );
-      ok(
-        !createButton.disabled,
-        "Create button should not be disabled initially"
-      );
+    await ContentTask.spawn(
+      browser,
+      [originTuple, i],
+      async ([aOriginTuple, index]) => {
+        let loginList = Cu.waiveXrays(
+          content.document.querySelector("login-list")
+        );
+        let createButton = loginList._createLoginButton;
+        ok(
+          ContentTaskUtils.is_hidden(loginList._blankLoginListItem),
+          "the blank login list item should be hidden initially"
+        );
+        ok(
+          !createButton.disabled,
+          "Create button should not be disabled initially"
+        );
 
-      createButton.click();
+        createButton.click();
+
+        ok(
+          ContentTaskUtils.is_visible(loginList._blankLoginListItem),
+          "the blank login list item should be visible after clicking on the create button"
+        );
+        ok(
+          createButton.disabled,
+          "Create button should be disabled after being clicked"
+        );
 
-      isnot(
-        content.getComputedStyle(loginList._blankLoginListItem).display,
-        "none",
-        "the blank login list item should be visible after clicking on the create button"
-      );
-      ok(
-        createButton.disabled,
-        "Create button should be disabled after being clicked"
-      );
+        let loginItem = Cu.waiveXrays(
+          content.document.querySelector("login-item")
+        );
 
-      let loginItem = Cu.waiveXrays(
-        content.document.querySelector("login-item")
-      );
+        let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
+        is(
+          ContentTaskUtils.is_hidden(cancelButton),
+          index == 0,
+          "cancel button should be hidden in create mode with no logins saved"
+        );
 
-      let originInput = loginItem.shadowRoot.querySelector(
-        "input[name='origin']"
-      );
-      let usernameInput = loginItem.shadowRoot.querySelector(
-        "input[name='username']"
-      );
-      let passwordInput = loginItem.shadowRoot.querySelector(
-        "input[name='password']"
-      );
+        let originInput = loginItem.shadowRoot.querySelector(
+          "input[name='origin']"
+        );
+        let usernameInput = loginItem.shadowRoot.querySelector(
+          "input[name='username']"
+        );
+        let passwordInput = loginItem.shadowRoot.querySelector(
+          "input[name='password']"
+        );
 
-      originInput.value = aOriginTuple[0];
-      usernameInput.value = "testuser1";
-      passwordInput.value = "testpass1";
+        originInput.value = aOriginTuple[0];
+        usernameInput.value = "testuser1";
+        passwordInput.value = "testpass1";
 
-      let saveChangesButton = loginItem.shadowRoot.querySelector(
-        ".save-changes-button"
-      );
-      saveChangesButton.click();
-    });
+        let saveChangesButton = loginItem.shadowRoot.querySelector(
+          ".save-changes-button"
+        );
+        saveChangesButton.click();
+      }
+    );
 
     info("waiting for login to get added to storage");
     await storageChangedPromised;
     info("login added to storage");
 
     storageChangedPromised = TestUtils.topicObserved(
       "passwordmgr-storage-changed",
       (_, data) => data == "modifyLogin"
     );
-    let expectedCount = i + 1;
-    await ContentTask.spawn(
-      browser,
-      { expectedCount, originTuple },
-      async ({ expectedCount: aExpectedCount, originTuple: aOriginTuple }) => {
-        ok(
-          !content.document.documentElement.classList.contains("no-logins"),
-          "Should no longer be in no logins view"
-        );
-        let loginList = Cu.waiveXrays(
-          content.document.querySelector("login-list")
-        );
-        ok(
-          !loginList.classList.contains("no-logins"),
-          "login-list should no longer be in no logins view"
+    await ContentTask.spawn(browser, originTuple, async aOriginTuple => {
+      ok(
+        !content.document.documentElement.classList.contains("no-logins"),
+        "Should no longer be in no logins view"
+      );
+      let loginList = Cu.waiveXrays(
+        content.document.querySelector("login-list")
+      );
+      ok(
+        !loginList.classList.contains("no-logins"),
+        "login-list should no longer be in no logins view"
+      );
+      ok(
+        ContentTaskUtils.is_hidden(loginList._blankLoginListItem),
+        "the blank login list item should be hidden after adding new login"
+      );
+      ok(
+        !loginList._createLoginButton.disabled,
+        "Create button shouldn't be disabled after exiting create login view"
+      );
+
+      let loginGuid = await ContentTaskUtils.waitForCondition(() => {
+        return loginList._loginGuidsSortedOrder.find(
+          guid => loginList._logins[guid].login.origin == aOriginTuple[1]
         );
-        is(
-          content.getComputedStyle(loginList._blankLoginListItem).display,
-          "none",
-          "the blank login list item should be hidden after adding new login"
-        );
-        ok(
-          !loginList._createLoginButton.disabled,
-          "Create button shouldn't be disabled after exiting create login view"
-        );
+      }, "Waiting for login to be displayed");
+      ok(loginGuid, "Expected login found in login-list");
 
-        let loginGuid = await ContentTaskUtils.waitForCondition(() => {
-          return loginList._loginGuidsSortedOrder.find(
-            guid => loginList._logins[guid].login.origin == aOriginTuple[1]
-          );
-        }, "Waiting for login to be displayed");
-        ok(loginGuid, "Expected login found in login-list");
-
-        let loginItem = Cu.waiveXrays(
-          content.document.querySelector("login-item")
-        );
-        is(loginItem._login.guid, loginGuid, "login-item should match");
+      let loginItem = Cu.waiveXrays(
+        content.document.querySelector("login-item")
+      );
+      is(loginItem._login.guid, loginGuid, "login-item should match");
 
-        let { login, listItem } = loginList._logins[loginGuid];
-        ok(
-          listItem.classList.contains("selected"),
-          "list item should be selected"
-        );
-        ok(
-          !!listItem,
-          `Stored login should only include the origin of the URL provided during creation (${
-            aOriginTuple[1]
-          })`
-        );
-        is(
-          login.username,
-          "testuser1",
-          "Stored login should have username provided during creation"
-        );
-        is(
-          login.password,
-          "testpass1",
-          "Stored login should have password provided during creation"
-        );
+      let { login, listItem } = loginList._logins[loginGuid];
+      ok(
+        listItem.classList.contains("selected"),
+        "list item should be selected"
+      );
+      is(
+        login.origin,
+        aOriginTuple[1],
+        "Stored login should only include the origin of the URL provided during creation"
+      );
+      is(
+        login.username,
+        "testuser1",
+        "Stored login should have username provided during creation"
+      );
+      is(
+        login.password,
+        "testpass1",
+        "Stored login should have password provided during creation"
+      );
 
-        let editButton = loginItem.shadowRoot.querySelector(".edit-button");
-        editButton.click();
+      let editButton = loginItem.shadowRoot.querySelector(".edit-button");
+      editButton.click();
 
-        let usernameInput = loginItem.shadowRoot.querySelector(
-          "input[name='username']"
-        );
-        let passwordInput = loginItem.shadowRoot.querySelector(
-          "input[name='password']"
-        );
-        usernameInput.value = "testuser2";
-        passwordInput.value = "testpass2";
+      let usernameInput = loginItem.shadowRoot.querySelector(
+        "input[name='username']"
+      );
+      let passwordInput = loginItem.shadowRoot.querySelector(
+        "input[name='password']"
+      );
+      usernameInput.value = "testuser2";
+      passwordInput.value = "testpass2";
 
-        let saveChangesButton = loginItem.shadowRoot.querySelector(
-          ".save-changes-button"
-        );
-        saveChangesButton.click();
-      }
-    );
+      let saveChangesButton = loginItem.shadowRoot.querySelector(
+        ".save-changes-button"
+      );
+      saveChangesButton.click();
+    });
 
     info("waiting for login to get modified in storage");
     await storageChangedPromised;
     info("login modified in storage");
 
     await ContentTask.spawn(browser, originTuple, async aOriginTuple => {
       let loginList = Cu.waiveXrays(
         content.document.querySelector("login-list")
       );
       let login = Object.values(loginList._logins).find(
         obj => obj.login.origin == aOriginTuple[1]
       ).login;
-      ok(
-        !!login,
+      is(
+        login.origin,
+        aOriginTuple[1],
         "Stored login should only include the origin of the URL provided during creation"
       );
       is(
         login.username,
         "testuser2",
         "Stored login should have modified username"
       );
       is(
         login.password,
         "testpass2",
         "Stored login should have modified password"
       );
     });
   }
+
+  await ContentTask.spawn(browser, testCases.length, async testCasesLength => {
+    let loginList = Cu.waiveXrays(content.document.querySelector("login-list"));
+    is(
+      loginList._loginGuidsSortedOrder.length,
+      5,
+      "login list should have a login per testcase"
+    );
+  });
 });
 
 add_task(async function test_cancel_create_login() {
   let browser = gBrowser.selectedBrowser;
   await ContentTask.spawn(browser, null, async () => {
     let loginList = Cu.waiveXrays(content.document.querySelector("login-list"));
     ok(
       loginList._selectedGuid,
       "there should be a selected guid before create mode"
     );
-    is(
-      content.getComputedStyle(loginList._blankLoginListItem).display,
-      "none",
+    ok(
+      ContentTaskUtils.is_hidden(loginList._blankLoginListItem),
       "the blank login list item should be hidden before create mode"
     );
 
     let createButton = content.document
       .querySelector("login-list")
       .shadowRoot.querySelector(".create-login-button");
     createButton.click();
 
     ok(
       !loginList._selectedGuid,
       "there should be no selected guid when in create mode"
     );
-    isnot(
-      content.getComputedStyle(loginList._blankLoginListItem).display,
-      "none",
+    ok(
+      ContentTaskUtils.is_visible(loginList._blankLoginListItem),
       "the blank login list item should be visible in create mode"
     );
 
     let loginItem = Cu.waiveXrays(content.document.querySelector("login-item"));
     let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
     cancelButton.click();
 
     ok(
       loginList._selectedGuid,
       "there should be a selected guid after canceling create mode"
     );
-    is(
-      content.getComputedStyle(loginList._blankLoginListItem).display,
-      "none",
+    ok(
+      ContentTaskUtils.is_hidden(loginList._blankLoginListItem),
       "the blank login list item should be hidden after canceling create mode"
     );
   });
 });
+
+add_task(
+  async function test_cancel_create_login_with_filter_showing_one_login() {
+    let browser = gBrowser.selectedBrowser;
+    await ContentTask.spawn(browser, null, async () => {
+      let loginList = Cu.waiveXrays(
+        content.document.querySelector("login-list")
+      );
+
+      let loginFilter = Cu.waiveXrays(
+        content.document.querySelector("login-filter")
+      );
+      loginFilter.value = "bugzilla.mozilla.org";
+      is(
+        loginList._list.querySelectorAll(
+          ".login-list-item[data-guid]:not([hidden])"
+        ).length,
+        1,
+        "filter should have one login showing"
+      );
+      let visibleLoginGuid = loginList.shadowRoot.querySelectorAll(
+        ".login-list-item[data-guid]:not([hidden])"
+      )[0].dataset.guid;
+
+      let createButton = loginList._createLoginButton;
+      createButton.click();
+
+      let loginItem = Cu.waiveXrays(
+        content.document.querySelector("login-item")
+      );
+      let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
+      ok(
+        ContentTaskUtils.is_visible(cancelButton),
+        "cancel button should be visible in create mode with one login showing"
+      );
+      cancelButton.click();
+
+      is(
+        loginFilter.value,
+        "bugzilla.mozilla.org",
+        "login-filter should not be cleared if there was a login in the list"
+      );
+      is(
+        loginList.shadowRoot.querySelectorAll(
+          ".login-list-item[data-guid]:not([hidden])"
+        )[0].dataset.guid,
+        visibleLoginGuid,
+        "the same login should still be visible"
+      );
+    });
+  }
+);
+
+add_task(async function test_cancel_create_login_with_logins_filtered_out() {
+  let browser = gBrowser.selectedBrowser;
+  await ContentTask.spawn(browser, null, async () => {
+    let loginFilter = Cu.waiveXrays(
+      content.document.querySelector("login-filter")
+    );
+    loginFilter.value = "XXX-no-logins-should-match-this-XXX";
+    await Promise.resolve();
+    let loginList = Cu.waiveXrays(content.document.querySelector("login-list"));
+    is(
+      loginList._list.querySelectorAll(
+        ".login-list-item[data-guid]:not([hidden])"
+      ).length,
+      0,
+      "filter should have no logins showing"
+    );
+
+    let createButton = loginList._createLoginButton;
+    createButton.click();
+
+    let loginItem = Cu.waiveXrays(content.document.querySelector("login-item"));
+    let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
+    ok(
+      ContentTaskUtils.is_visible(cancelButton),
+      "cancel button should be visible in create mode with no logins showing"
+    );
+    cancelButton.click();
+    await Promise.resolve();
+
+    is(
+      loginFilter.value,
+      "",
+      "login-filter should be cleared if there were no logins in the list"
+    );
+    let visibleLoginItems = loginList.shadowRoot.querySelectorAll(
+      ".login-list-item[data-guid]:not([hidden])"
+    );
+    is(
+      visibleLoginItems.length,
+      5,
+      "all logins should be visible with blank filter"
+    );
+    is(
+      loginList._selectedGuid,
+      visibleLoginItems[0].dataset.guid,
+      "the first item in the list should be selected"
+    );
+  });
+});
--- a/testing/mochitest/BrowserTestUtils/ContentTaskUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/ContentTaskUtils.jsm
@@ -37,46 +37,41 @@ var ContentTaskUtils = {
     if (style.display == "none") {
       return true;
     }
     if (style.visibility != "visible") {
       return true;
     }
 
     // Hiding a parent element will hide all its children
-    if (element.parentNode != element.ownerDocument) {
+    if (
+      element.parentNode != element.ownerDocument &&
+      element.parentNode.nodeType != Node.DOCUMENT_FRAGMENT_NODE
+    ) {
       return ContentTaskUtils.is_hidden(element.parentNode);
     }
 
+    // Walk up the shadow DOM if we've reached the top of the shadow root
+    if (element.parentNode.host) {
+      return ContentTaskUtils.is_hidden(element.parentNode.host);
+    }
+
     return false;
   },
 
   /**
    * Checks if a DOM element is visible.
    *
    * @param {Element} element
    *        The element which is to be checked.
    *
    * @return {boolean}
    */
   is_visible(element) {
-    let style = element.ownerDocument.defaultView.getComputedStyle(element);
-    if (style.display == "none") {
-      return false;
-    }
-    if (style.visibility != "visible") {
-      return false;
-    }
-
-    // Hiding a parent element will hide all its children
-    if (element.parentNode != element.ownerDocument) {
-      return ContentTaskUtils.is_visible(element.parentNode);
-    }
-
-    return true;
+    return !this.is_hidden(element);
   },
 
   /**
    * Will poll a condition function until it returns true.
    *
    * @param condition
    *        A condition function that must return true or false. If the
    *        condition ever throws, this is also treated as a false.