Skip to content

Commit

Permalink
Avoid length-based filter in inferSelectors (#2905)
Browse files Browse the repository at this point in the history
  • Loading branch information
fregante authored Mar 8, 2022
1 parent c3604af commit ac33400
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 27 deletions.
17 changes: 0 additions & 17 deletions src/contentScript/nativeEditor/infer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
inferButtonHTML,
inferPanelHTML,
inferSelectors,
isSelectorPotentiallyUseful,
safeCssSelector,
} from "@/contentScript/nativeEditor/infer";
import { PIXIEBRIX_DATA_ATTR, EXTENSION_POINT_DATA_ATTR } from "@/common";
Expand Down Expand Up @@ -354,22 +353,6 @@ test("getSelectorPreference: matches expected sorting", () => {
expect(getSelectorPreference(selector)).toBe(selector.length);
});

test("isSelectorPotentiallyUseful", () => {
const fn = isSelectorPotentiallyUseful;
expect(fn(".navItem")).toBeTruthy();
expect(fn(".birdsArentReal")).toBeTruthy();
expect(fn('[aria-label="Click elsewhere"]')).toBeTruthy();

// Always allow IDs
expect(fn("#yes")).toBeTruthy();

// Exclude utility classes
expect(fn(".p-1")).toBeFalsy();

// Exclude some short random classes
expect(fn("._d3f32f")).toBeFalsy();
});

describe("inferSelectors", () => {
const expectSelectors = (selectors: string[], body: string) => {
document.body.innerHTML = body;
Expand Down
12 changes: 2 additions & 10 deletions src/contentScript/nativeEditor/infer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { uniq, sortBy, unary, intersection } from "lodash";
import { uniq, sortBy, unary, intersection, isEmpty } from "lodash";
import { getCssSelector } from "css-selector-generator";
import { isNullOrBlank, mostCommonElement, matchesAnyPattern } from "@/utils";
import { BusinessError } from "@/errors";
Expand Down Expand Up @@ -124,14 +124,6 @@ export function getSelectorPreference(selector: string): number {
return selector.length;
}

/** Excludes empty or short selectors (must have more than 3 letters, no numbers) */
export function isSelectorPotentiallyUseful(selector: string): boolean {
// Remove the non-letter characters, and then compare the number of remaining letter characters
return (
selector.startsWith("#") || selector.replace(/[^a-z]/gi, "").length > 3
);
}

function outerHTML(element: Element | string): string {
if (typeof element === "string") {
return element;
Expand Down Expand Up @@ -603,7 +595,7 @@ export function inferSelectors(
makeSelector(["id", "tag", "attribute", "nthchild"]),
makeSelector(["id", "tag", "attribute"]),
makeSelector(),
]).filter((x) => isSelectorPotentiallyUseful(x));
]).filter((x) => !isEmpty(x));

return sortBy(generatedSelectors, getSelectorPreference);
}
Expand Down

0 comments on commit ac33400

Please sign in to comment.