Simplify exclusions (#724)

* Change: exclusion should match urlWithoutParams

Revert to eight04's initial two commits in #681 which make exclusion toggles domain and singular URLs only, plus reincorporate the js menu height calculation.

* Change: drop excludeStyleByUrlRedundant plus menu height

* menu item text

* Make exclusion rules work like match pattern and handle invalid URLs

* Exclude rules in tooltips

* Remove leftover code

* Cross-browser overflow consistency

_locales/en/messages.json

@@ -321,9 +321,6 @@
   "excludeStyleByUrlLabel": {
     "message": "Exclude the current URL"
   },
-  "excludeStyleByUrlRedundant": {
-    "message": "The current URL is the domain page"
-  },
   "exportLabel": {
     "message": "Export",
     "description": "Label for the button to export a style ('edit' page) or all styles ('manage' page)"

background/style-manager.js

@@ -43,7 +43,22 @@ const styleManager = (() => {
   const BAD_MATCHER = {test: () => false};
   const compileRe = createCompiler(text => `^(${text})$`);
   const compileSloppyRe = createCompiler(text => `^${text}$`);
-  const compileExclusion = createCompiler(buildGlob);
+  const compileExclusion = createCompiler(buildExclusion);
+
+  const DUMMY_URL = {
+    hash: '',
+    host: '',
+    hostname: '',
+    href: '',
+    origin: '',
+    password: '',
+    pathname: '',
+    port: '',
+    protocol: '',
+    search: '',
+    searchParams: new URLSearchParams(),
+    username: ''
+  };
 
   handleLivePreviewConnections();
 
@@ -280,7 +295,7 @@ const styleManager = (() => {
         cache.maybeMatch.add(data.id);
         continue;
       }
-      const code = getAppliedCode(url, data);
+      const code = getAppliedCode(createMatchQuery(url), data);
       if (!code) {
         excluded.add(url);
         delete cache.sections[data.id];
@@ -346,11 +361,12 @@ const styleManager = (() => {
     const result = [];
     const datas = !id ? [...styles.values()].map(s => s.data) :
       styles.has(id) ? [styles.get(id).data] : [];
+    const query = createMatchQuery(url);
     for (const data of datas) {
       let excluded = false;
       let sloppy = false;
       let sectionMatched = false;
-      const match = urlMatchStyle(url, data);
+      const match = urlMatchStyle(query, data);
       // TODO: enable this when the function starts returning false
       // if (match === false) {
         // continue;
@@ -362,7 +378,7 @@ const styleManager = (() => {
         if (styleCodeEmpty(section.code)) {
           continue;
         }
-        const match = urlMatchSection(url, section);
+        const match = urlMatchSection(query, section);
         if (match) {
           if (match === 'sloppy') {
             sloppy = true;
@@ -407,8 +423,9 @@ const styleManager = (() => {
     return cache.sections;
 
     function buildCache(styleList) {
+      const query = createMatchQuery(url);
       for (const {appliesTo, data, preview} of styleList) {
-        const code = getAppliedCode(url, preview || data);
+        const code = getAppliedCode(query, preview || data);
         if (code) {
           cache.sections[data.id] = {
             id: data.id,
@@ -420,13 +437,13 @@ const styleManager = (() => {
     }
   }
 
-  function getAppliedCode(url, data) {
+  function getAppliedCode(query, data) {
-    if (urlMatchStyle(url, data) !== true) {
+    if (urlMatchStyle(query, data) !== true) {
       return;
     }
     const code = [];
     for (const section of data.sections) {
-      if (urlMatchSection(url, section) === true && !styleCodeEmpty(section.code)) {
+      if (urlMatchSection(query, section) === true && !styleCodeEmpty(section.code)) {
         code.push(section.code);
       }
     }
@@ -452,8 +469,11 @@ const styleManager = (() => {
     });
   }
 
-  function urlMatchStyle(url, style) {
+  function urlMatchStyle(query, style) {
-    if (style.exclusions && style.exclusions.some(e => compileExclusion(e).test(url))) {
+    if (
+      style.exclusions &&
+      style.exclusions.some(e => compileExclusion(e).test(query.urlWithoutParams))
+    ) {
       return 'excluded';
     }
     if (!style.enabled) {
@@ -462,12 +482,14 @@ const styleManager = (() => {
     return true;
   }
 
-  function urlMatchSection(url, section) {
+  function urlMatchSection(query, section) {
-    const domain = getDomain(url);
+    if (
-    if (section.domains && section.domains.some(d => d === domain || domain.endsWith(`.${d}`))) {
+      section.domains &&
+      section.domains.some(d => d === query.domain || query.domain.endsWith(`.${d}`))
+    ) {
       return true;
     }
-    if (section.urlPrefixes && section.urlPrefixes.some(p => url.startsWith(p))) {
+    if (section.urlPrefixes && section.urlPrefixes.some(p => query.url.startsWith(p))) {
       return true;
     }
     // as per spec the fragment portion is ignored in @-moz-document:
@@ -475,12 +497,12 @@ const styleManager = (() => {
     // but the spec is outdated and doesn't account for SPA sites
     // so we only respect it for `url()` function
     if (section.urls && (
-      section.urls.includes(url) ||
+      section.urls.includes(query.url) ||
-      section.urls.includes(getUrlNoHash(url))
+      section.urls.includes(query.urlWithoutHash)
     )) {
       return true;
     }
-    if (section.regexps && section.regexps.some(r => compileRe(r).test(url))) {
+    if (section.regexps && section.regexps.some(r => compileRe(r).test(query.url))) {
       return true;
     }
     /*
@@ -489,7 +511,7 @@ const styleManager = (() => {
     We'll detect styles that abuse the bug by finding the sections that
     would have been applied by Stylish but not by us as we follow the spec.
     */
-    if (section.regexps && section.regexps.some(r => compileSloppyRe(r).test(url))) {
+    if (section.regexps && section.regexps.some(r => compileSloppyRe(r).test(query.url))) {
       return 'sloppy';
     }
     // TODO: check for invalid regexps?
@@ -522,16 +544,22 @@ const styleManager = (() => {
     };
   }
 
-  function buildGlob(text) {
+  function compileGlob(text) {
-    return '^' + escapeRegExp(text).replace(/\\\\\\\*|\\\*/g, m => m.length > 2 ? m : '.*') + '$';
+    return escapeRegExp(text).replace(/\\\\\\\*|\\\*/g, m => m.length > 2 ? m : '.*');
   }
 
-  function getDomain(url) {
+  function buildExclusion(text) {
-    return url.match(/^[\w-]+:\/+(?:[\w:-]+@)?([^:/#]+)/)[1];
+    // match pattern
+    const match = text.match(/^(\*|[\w-]+):\/\/(\*\.)?([\w.]+\/.*)/);
+    if (!match) {
+      return '^' + compileGlob(text) + '$';
     }
-
+    return '^' +
-  function getUrlNoHash(url) {
+      (match[1] === '*' ? '[\\w-]+' : match[1]) +
-    return url.split('#')[0];
+      '://' +
+      (match[2] ? '(?:[\\w.]+\\.)?' : '') +
+      compileGlob(match[3]) +
+      '$';
   }
 
   // The md5Url provided by USO includes a duplicate "update" subdomain (see #523),
@@ -541,4 +569,41 @@ const styleManager = (() => {
       style.md5Url = style.md5Url.replace('update.update.userstyles', 'update.userstyles');
     }
   }
+
+  function createMatchQuery(url) {
+    let urlWithoutHash;
+    let urlWithoutParams;
+    let domain;
+    return {
+      url,
+      get urlWithoutHash() {
+        if (!urlWithoutHash) {
+          urlWithoutHash = url.split('#')[0];
+        }
+        return urlWithoutHash;
+      },
+      get urlWithoutParams() {
+        if (!urlWithoutParams) {
+          const u = createURL(url);
+          urlWithoutParams = u.origin + u.pathname;
+        }
+        return urlWithoutParams;
+      },
+      get domain() {
+        if (!domain) {
+          const u = createURL(url);
+          domain = u.hostname;
+        }
+        return domain;
+      }
+    };
+  }
+
+  function createURL(url) {
+    try {
+      return new URL(url);
+    } catch (err) {
+      return DUMMY_URL;
+    }
+  }
 })();

popup/popup.css

@@ -26,6 +26,10 @@ body {
   margin: 0;
 }
 
+html, body:not(.search-results-shown) {
+  overflow: hidden;
+}
+
 .firefox body {
   color: #000;
   background-color: #fff;
@@ -327,8 +331,7 @@ a.configure[target="_blank"] .svg-icon.config {
   opacity: 0;
 }
 .entry.menu-active .menu {
-  /* FIXME: avoid hard coded height */
+  height: var(--menu-height, 0px);
-  height: 72px;
   opacity: 1;
 }
 /* accessibility */
@@ -336,12 +339,12 @@ a.configure[target="_blank"] .svg-icon.config {
   display: none;
   border: none;
   align-items: center;
-  padding: 0 0 0 20px;
+  padding: 3px 0 3px 20px;
-  height: 24px;
   background: none;
   text-decoration: none;
+  flex: none;
 }
-.entry.menu-active.accessible-items .menu-item {
+.entry.menu-active .menu-item {
   display: flex;
 }
 .entry .menu-item.delete {
@@ -360,6 +363,7 @@ a.configure[target="_blank"] .svg-icon.config {
 }
 .entry .menu-icon {
   width: 26px;
+  flex-shrink: 0;
 }
 .entry .menu-icon > * {
   display: block;

popup/popup.js

@@ -335,16 +335,10 @@ function createStyleElement(style) {
   entry.classList.toggle('regexp-partial', style.sloppy);
 
   $('.exclude-by-domain-checkbox', entry).checked = styleExcluded(style, 'domain');
+  $('.exclude-by-url-checkbox', entry).checked = styleExcluded(style, 'url');
 
-  const excludeByUrlCheckbox = $('.exclude-by-url-checkbox', entry);
+  $('.exclude-by-domain', entry).title = getExcludeRule('domain');
-  const isRedundant = getExcludeRule('domain') === getExcludeRule('url');
+  $('.exclude-by-url', entry).title = getExcludeRule('url');
-  excludeByUrlCheckbox.checked = !isRedundant && styleExcluded(style, 'url');
-  excludeByUrlCheckbox.disabled = isRedundant;
-
-  const excludeByUrlLabel = $('.exclude-by-url', entry);
-  excludeByUrlLabel.classList.toggle('disabled', isRedundant);
-  excludeByUrlLabel.title = isRedundant ?
-    chrome.i18n.getMessage('excludeStyleByUrlRedundant') : '';
 
   return entry;
 }
@@ -358,10 +352,16 @@ function styleExcluded({exclusions}, type) {
 }
 
 function getExcludeRule(type) {
+  const u = new URL(tabURL);
   if (type === 'domain') {
-    return new URL(tabURL).origin + '/*';
+    return u.origin + '/*';
   }
-  return tabURL + '*';
+  // current page
+  return escapeGlob(u.origin + u.pathname);
+}
+
+function escapeGlob(text) {
+  return text.replace(/\*/g, '\\*');
 }
 
 Object.assign(handleEvent, {
@@ -399,9 +399,8 @@ Object.assign(handleEvent, {
   toggleMenu(event) {
     const entry = handleEvent.getClickedStyleElement(event);
     entry.classList.toggle('menu-active');
-    setTimeout(() => {
+    const menu = entry.querySelector('.menu');
-      entry.classList.toggle('accessible-items');
+    menu.style.setProperty('--menu-height', menu.scrollHeight + 'px');
-    }, 250);
     event.preventDefault();
   },