Bug 1576100 - Ensure correct document order of category items for keyboard nav. r=johannh
authorNihanth Subramanya <nhnt11@gmail.com>
Fri, 30 Aug 2019 15:29:39 +0000
changeset 554647 274f8d9b14b6eb32368010c386a59c16fcc6ccc4
parent 554646 9a59db93153408008285cac6cb17bf9d84b86aca
child 554648 202b771061fb165810bdc2e9f0f220535f43e2e0
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1576100
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 1576100 - Ensure correct document order of category items for keyboard nav. r=johannh Differential Revision: https://phabricator.services.mozilla.com/D43932
browser/base/content/browser-siteProtections.js
browser/base/content/test/siteProtections/browser_protections_UI.js
browser/components/controlcenter/content/protectionsPanel.inc.xul
browser/themes/shared/controlcenter/panel.inc.css
--- a/browser/base/content/browser-siteProtections.js
+++ b/browser/base/content/browser-siteProtections.js
@@ -1299,16 +1299,34 @@ var gProtectionsHandler = {
     ));
   },
   get _protectionsPopupTPSwitch() {
     delete this._protectionsPopupTPSwitch;
     return (this._protectionsPopupTPSwitch = document.getElementById(
       "protections-popup-tp-switch"
     ));
   },
+  get _protectionsPopupBlockingHeader() {
+    delete this._protectionsPopupBlockingHeader;
+    return (this._protectionsPopupBlockingHeader = document.getElementById(
+      "protections-popup-blocking-section-header"
+    ));
+  },
+  get _protectionsPopupNotBlockingHeader() {
+    delete this._protectionsPopupNotBlockingHeader;
+    return (this._protectionsPopupNotBlockingHeader = document.getElementById(
+      "protections-popup-not-blocking-section-header"
+    ));
+  },
+  get _protectionsPopupNotFoundHeader() {
+    delete this._protectionsPopupNotFoundHeader;
+    return (this._protectionsPopupNotFoundHeader = document.getElementById(
+      "protections-popup-not-found-section-header"
+    ));
+  },
   get _protectionsPopupSettingsButton() {
     delete this._protectionsPopupSettingsButton;
     return (this._protectionsPopupSettingsButton = document.getElementById(
       "protections-popup-settings-button"
     ));
   },
   get _protectionsPopupFooter() {
     delete this._protectionsPopupFooter;
@@ -1416,20 +1434,21 @@ var gProtectionsHandler = {
   // with at least the following two properties:
   //  - enabled: Whether the blocker is currently turned on.
   //  - isDetected(state): Given a content blocking state, whether the blocker has
   //                       either allowed or blocked elements.
   //  - categoryItem: The DOM item that represents the entry in the category list.
   //
   // It may also contain an init() and uninit() function, which will be called
   // on gProtectionsHandler.init() and gProtectionsHandler.uninit().
+  // The buttons in the protections panel will appear in the same order as this array.
   blockers: [
-    TrackingProtection,
     SocialTracking,
     ThirdPartyCookies,
+    TrackingProtection,
     Fingerprinting,
     Cryptomining,
   ],
 
   init() {
     this.animatedIcon.addEventListener("animationend", () =>
       this.iconBox.removeAttribute("animate")
     );
@@ -1495,64 +1514,44 @@ var gProtectionsHandler = {
     switchToTabHavingURI("about:protections", true, {
       replaceQueryString: true,
       relatedToCurrent,
       triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
     });
   },
 
   async showTrackersSubview(event) {
-    if (event.target.classList.contains("notFound")) {
-      return;
-    }
-
     await TrackingProtection.updateSubView();
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-trackersView"
     );
   },
 
   async showSocialblockerSubview(event) {
-    if (event.target.classList.contains("notFound")) {
-      return;
-    }
-
     await SocialTracking.updateSubView();
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-socialblockView"
     );
   },
 
   async showCookiesSubview(event) {
-    if (event.target.classList.contains("notFound")) {
-      return;
-    }
-
     await ThirdPartyCookies.updateSubView();
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-cookiesView"
     );
   },
 
   async showFingerprintersSubview(event) {
-    if (event.target.classList.contains("notFound")) {
-      return;
-    }
-
     await Fingerprinting.updateSubView();
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-fingerprintersView"
     );
   },
 
   async showCryptominersSubview(event) {
-    if (event.target.classList.contains("notFound")) {
-      return;
-    }
-
     await Cryptomining.updateSubView();
     this._protectionsPopupMultiView.showSubView(
       "protections-popup-cryptominersView"
     );
   },
 
   recordClick(object, value = null) {
     Services.telemetry.recordEvent(
@@ -1608,16 +1607,18 @@ var gProtectionsHandler = {
         "open",
         "true"
       );
 
       // Insert the info message if needed. This will be shown once and then
       // remain collapsed.
       ToolbarPanelHub.insertProtectionPanelMessage(event);
 
+      this.reorderCategoryItems();
+
       if (!event.target.hasAttribute("toast")) {
         Services.telemetry.recordEvent(
           "security.ui.protectionspopup",
           "open",
           "protections_popup"
         );
       }
     }
@@ -1729,16 +1730,21 @@ var gProtectionsHandler = {
       // runs on tab switch, so we can avoid associating the data with the document directly.
       blocker.activated = blocker.isBlocking(event);
       let detected = blocker.isDetected(event);
       blocker.categoryItem.classList.toggle("notFound", !detected);
       anyDetected = anyDetected || detected;
       anyBlocking = anyBlocking || blocker.activated;
     }
 
+    this._categoryItemOrderInvalidated = true;
+    if (this._protectionsPopup.state == "open") {
+      this.reorderCategoryItems();
+    }
+
     if (anyDetected) {
       this.noTrackersDetectedDescription.hidden = true;
     }
 
     // Check whether the user has added an exception for this site.
     let hasException = ContentBlockingAllowList.includes(
       gBrowser.selectedBrowser
     );
@@ -1918,16 +1924,74 @@ var gProtectionsHandler = {
         ])
       );
     }
 
     // Update the tooltip of the blocked tracker counter.
     this.maybeUpdateEarliestRecordedDateTooltip();
   },
 
+  /*
+   * This function sorts the category items into the Blocked/Allowed/None Detected
+   * sections. It's called immediately in onContentBlockingEvent if the popup
+   * is presently open. Otherwise, the next time the popup is shown.
+   */
+  reorderCategoryItems() {
+    if (!this._categoryItemOrderInvalidated) {
+      return;
+    }
+
+    delete this._categoryItemOrderInvalidated;
+
+    // Hide all the headers to start with.
+    this._protectionsPopupBlockingHeader.hidden = true;
+    this._protectionsPopupNotBlockingHeader.hidden = true;
+    this._protectionsPopupNotFoundHeader.hidden = true;
+
+    for (let { categoryItem } of this.blockers) {
+      if (categoryItem.classList.contains("notFound")) {
+        // Add the item to the bottom of the list. This will be under
+        // the "None Detected" section.
+        categoryItem.parentNode.insertAdjacentElement(
+          "beforeend",
+          categoryItem
+        );
+        categoryItem.setAttribute("disabled", true);
+        // We have an undetected category, show the header.
+        this._protectionsPopupNotFoundHeader.hidden = false;
+        continue;
+      }
+
+      // Clear the disabled attribute in case we are moving the item out of
+      // "None Detected"
+      categoryItem.removeAttribute("disabled");
+
+      if (categoryItem.classList.contains("blocked") && !this.hasException) {
+        // Add the item just above the "Allowed" section - this will be the
+        // bottom of the "Blocked" section.
+        categoryItem.parentNode.insertBefore(
+          categoryItem,
+          this._protectionsPopupNotBlockingHeader
+        );
+        // We have a blocking category, show the header.
+        this._protectionsPopupBlockingHeader.hidden = false;
+        continue;
+      }
+
+      // Add the item just above the "None Detected" section - this will be the
+      // bottom of the "Allowed" section.
+      categoryItem.parentNode.insertBefore(
+        categoryItem,
+        this._protectionsPopupNotFoundHeader
+      );
+      // We have an allowing category, show the header.
+      this._protectionsPopupNotBlockingHeader.hidden = false;
+    }
+  },
+
   disableForCurrentPage(shouldReload = true) {
     ContentBlockingAllowList.add(gBrowser.selectedBrowser);
     if (shouldReload) {
       PanelMultiView.hidePopup(this._protectionsPopup);
       BrowserReload();
     }
   },
 
--- a/browser/base/content/test/siteProtections/browser_protections_UI.js
+++ b/browser/base/content/test/siteProtections/browser_protections_UI.js
@@ -539,18 +539,18 @@ add_task(async function testSubViewTelem
     ["protections-popup-category-socialblock", "social"],
     ["protections-popup-category-cookies", "cookies"],
     ["protections-popup-category-cryptominers", "cryptominers"],
     ["protections-popup-category-fingerprinters", "fingerprinters"],
   ].map(item => [document.getElementById(item[0]), item[1]]);
 
   for (let [item, telemetryId] of items) {
     await BrowserTestUtils.withNewTab("http://www.example.com", async () => {
+      item.classList.remove("notFound"); // Force visible for test
       await openProtectionsPanel();
-      item.classList.remove("notFound"); // Force visible for test
       let viewShownEvent = BrowserTestUtils.waitForEvent(
         gProtectionsHandler._protectionsPopupMultiView,
         "ViewShown"
       );
       item.click();
       let panelView = (await viewShownEvent).originalTarget;
       checkClickTelemetry(telemetryId);
       let prefsTabPromise = BrowserTestUtils.waitForNewTab(
--- a/browser/components/controlcenter/content/protectionsPanel.inc.xul
+++ b/browser/components/controlcenter/content/protectionsPanel.inc.xul
@@ -58,59 +58,59 @@
       <!-- Tracking Protection Section -->
       <vbox id="tracking-protection-container" class="protections-popup-section">
         <description id="protections-popup-no-trackers-found-description">&protections.noTrackersFound.description;</description>
         <tooltip id="protections-popup-not-blocking-why-etp-on-tooltip">&protections.notBlocking.why.etpOn.tooltip;</tooltip>
         <tooltip id="protections-popup-not-blocking-why-etp-off-tooltip">&protections.notBlocking.why.etpOff.tooltip;</tooltip>
         <vbox id="protections-popup-content" flex="1">
           <vbox id="protections-popup-category-list">
             <toolbarbutton id="protections-popup-category-tracking-protection"
-                           onclick="gProtectionsHandler.showTrackersSubview(event); gProtectionsHandler.recordClick('trackers');"
+                           oncommand="gProtectionsHandler.showTrackersSubview(event); gProtectionsHandler.recordClick('trackers');"
                            class="protections-popup-category" align="center"
                            wrap="true">
               <image class="protections-popup-category-icon tracking-protection-icon"/>
               <label flex="1" class="protections-popup-category-label">&contentBlocking.trackingProtection4.label;</label>
             </toolbarbutton>
             <toolbarbutton id="protections-popup-category-socialblock"
-                           onclick="gProtectionsHandler.showSocialblockerSubview(event); gProtectionsHandler.recordClick('social');"
+                           oncommand="gProtectionsHandler.showSocialblockerSubview(event); gProtectionsHandler.recordClick('social');"
                            class="protections-popup-category" align="center">
               <image class="protections-popup-category-icon socialblock-icon"/>
               <label flex="1"
               class="protections-popup-category-label">&contentBlocking.socialblock.label;</label>
             </toolbarbutton>
             <toolbarbutton id="protections-popup-category-cookies"
-                           onclick="gProtectionsHandler.showCookiesSubview(event); gProtectionsHandler.recordClick('cookies');"
+                           oncommand="gProtectionsHandler.showCookiesSubview(event); gProtectionsHandler.recordClick('cookies');"
                            class="protections-popup-category" align="center"
                            wrap="true">
               <image class="protections-popup-category-icon thirdpartycookies-icon"/>
               <label flex="1" id="protections-popup-cookies-category-label-disabled"
                      class="protections-popup-category-label">&contentBlocking.cookies2.label;</label>
               <label flex="1" id="protections-popup-cookies-category-label-enabled"
                      class="protections-popup-category-label"
                      hidden="true"></label>
             </toolbarbutton>
             <toolbarbutton id="protections-popup-category-cryptominers"
-                           onclick="gProtectionsHandler.showCryptominersSubview(event); gProtectionsHandler.recordClick('cryptominers');"
+                           oncommand="gProtectionsHandler.showCryptominersSubview(event); gProtectionsHandler.recordClick('cryptominers');"
                            class="protections-popup-category" align="center"
                            wrap="true">
               <image class="protections-popup-category-icon cryptominers-icon"/>
               <label flex="1" class="protections-popup-category-label">&contentBlocking.cryptominers.label;</label>
             </toolbarbutton>
             <toolbarbutton id="protections-popup-category-fingerprinters"
-                           onclick="gProtectionsHandler.showFingerprintersSubview(event); gProtectionsHandler.recordClick('fingerprinters');"
+                           oncommand="gProtectionsHandler.showFingerprintersSubview(event); gProtectionsHandler.recordClick('fingerprinters');"
                            class="protections-popup-category" align="center"
                            wrap="true">
               <image class="protections-popup-category-icon fingerprinters-icon"/>
               <label flex="1" class="protections-popup-category-label">&contentBlocking.fingerprinters.label;</label>
             </toolbarbutton>
             <description id="protections-popup-blocking-section-header"
                          role="heading"
                          aria-level="2">&protections.blocking2.label;</description>
-            <hbox id="protections-popup-not-blocking-section-header">
-              <description id="protections-popup-not-blocking-section-description">&protections.notBlocking2.label;</description>
+            <hbox id="protections-popup-not-blocking-section-header" flex="1">
+              <description id="protections-popup-not-blocking-section-description" flex="1">&protections.notBlocking2.label;</description>
               <label id="protections-popup-not-blocking-section-why"
                      class="text-link"
                      onmouseover="document.getElementById(event.target.tooltip).openPopup(event.target);"
                      onmouseout="document.getElementById(event.target.tooltip).hidePopup()">&protections.notBlocking.why.label;</label>
             </hbox>
             <description id="protections-popup-not-found-section-header"
                          role="heading"
                          aria-level="2">&protections.notFound.label;</description>
--- a/browser/themes/shared/controlcenter/panel.inc.css
+++ b/browser/themes/shared/controlcenter/panel.inc.css
@@ -672,17 +672,16 @@ description#identity-popup-content-verif
 }
 
 #protections-popup-blocking-section-header,
 #protections-popup-not-blocking-section-header,
 #protections-popup-not-found-section-header {
   margin: 0;
   padding: var(--vertical-section-padding) var(--horizontal-padding);
   color: #737373;
-  display: none;
 }
 
 :root[lwt-popup-brighttext] #protections-popup-no-trackers-found-description,
 :root[lwt-popup-brighttext] #protections-popup-blocking-section-header,
 :root[lwt-popup-brighttext] #protections-popup-not-blocking-section-header {
   color: #f9f9fa;
 }
 
@@ -694,139 +693,53 @@ description#identity-popup-content-verif
  * with five different category items distributed between them at runtime.
  * To achieve this, we use a grid layout with 12 rows: one row for each header
  * label and five rows in each section for the items.
  * Items with the "blocked" class are assigned rows 2-6, and those without
  * are assigned rows 8-12, with the headers taking rows 1 and 7.
  */
 
 #protections-popup-category-list {
-  display: grid;
   padding: 0;
   margin: 0;
 }
 
 #protections-popup-no-trackers-found-description:not([hidden]) ~ #protections-popup-content {
   display: none;
 }
 
-#protections-popup-blocking-section-header,
-#protections-popup-not-blocking-section-header,
-#protections-popup-not-found-section-header {
-  display: none;
-}
-
-#protections-popup-blocking-section-header {
-  grid-row: 1;
-}
-
-#protections-popup:not([hasException]) .protections-popup-category.blocked:not(.notFound) ~ #protections-popup-blocking-section-header,
-.protections-popup-category.notFound ~ #protections-popup-not-found-section-header,
-.protections-popup-category:not(.blocked):not(.notFound) ~ #protections-popup-not-blocking-section-header,
-#protections-popup[hasException] .protections-popup-category:not(.notFound) ~ #protections-popup-not-blocking-section-header {
-  display: flex;
-}
-
-#protections-popup:not([hasException]) #protections-popup-category-socialblock.blocked {
-  grid-row: 2;
-}
-
-#protections-popup:not([hasException]) #protections-popup-category-cookies.blocked {
-  grid-row: 3;
-}
-
-#protections-popup:not([hasException]) #protections-popup-category-tracking-protection.blocked {
-  grid-row: 4;
-}
-
-#protections-popup:not([hasException]) #protections-popup-category-fingerprinters.blocked {
-  grid-row: 5;
-}
-
-#protections-popup:not([hasException]) #protections-popup-category-cryptominers.blocked {
-  grid-row: 6;
-}
-
-#protections-popup-not-blocking-section-header {
-  grid-row: 7;
-}
-
 #protections-popup-not-blocking-section-description {
   flex: -moz-available;
   margin: 0;
 }
 
 #protections-popup-not-blocking-section-why {
   margin: 0;
 }
 
 #protections-popup-not-blocking-section-why:hover {
   background-color: var(--arrowpanel-dimmed);
   outline: 4px solid var(--arrowpanel-dimmed);
   text-decoration: none;
 }
 
-#protections-popup-category-socialblock {
-  grid-row: 8;
-}
-
-#protections-popup-category-cookies {
-  grid-row: 9;
-}
-
-#protections-popup-category-tracking-protection {
-  grid-row: 10;
-}
-
-#protections-popup-category-fingerprinters {
-  grid-row: 11;
-}
-
-#protections-popup-category-cryptominers {
-  grid-row: 12;
-}
-
-#protections-popup-not-found-section-header {
-  grid-row: 13;
-}
-
 .protections-popup-category.notFound {
   color: var(--panel-disabled-color);
   fill: var(--panel-disabled-color);
 }
 
 .protections-popup-category.notFound:hover {
   background: none;
 }
 
 /* Hide the arrow for not found items */
 .protections-popup-category.notFound::after {
   display: none;
 }
 
-#protections-popup-category-socialblock.notFound {
-  grid-row: 14 !important;
-}
-
-#protections-popup-category-cookies.notFound {
-  grid-row: 15 !important;
-}
-
-#protections-popup-category-tracking-protection.notFound {
-  grid-row: 16 !important;
-}
-
-#protections-popup-category-fingerprinters.notFound {
-  grid-row: 17 !important;
-}
-
-#protections-popup-category-cryptominers.notFound {
-  grid-row: 18 !important;
-}
-
 .tracking-protection-icon {
   list-style-image: url(chrome://browser/skin/controlcenter/tracker-image.svg);
 }
 
 .socialblock-icon {
   list-style-image: url(chrome://browser/skin/controlcenter/socialblock.svg);
 }