Merge pull request #324 from ezzak/improve_content_test

Improve manifest data attribute check to ensure both version/type are set
facebookincubator · May 10, 2024 · 58112e2 · 58112e2
2 parents f793dc4 + 64e0dff
commit 58112e2
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 9 deletions.
diff --git a/src/js/__tests__/contentUtils-test.js b/src/js/__tests__/contentUtils-test.js
@@ -31,7 +31,7 @@ describe('contentUtils', () => {
       const fakeScriptNode = {
         src: fakeUrl,
         getAttribute: () => {
-          return 'data-btmanifest';
+          return '123_main';
         },
       };
       storeFoundJS(fakeScriptNode);
@@ -44,7 +44,7 @@ describe('contentUtils', () => {
       const fakeScriptNode = {
         src: fakeUrl,
         getAttribute: () => {
-          return 'data-btmanifest';
+          return '123_main';
         },
       };
       storeFoundJS(fakeScriptNode);
@@ -68,7 +68,7 @@ describe('contentUtils', () => {
     it('should store any script elements we find', () => {
       const fakeElement = {
         getAttribute: () => {
-          return 'data-btmanifest';
+          return '123_main';
         },
         childNodes: [],
         nodeName: 'SCRIPT',
@@ -128,7 +128,7 @@ describe('contentUtils', () => {
           },
           {
             getAttribute: () => {
-              return 'data-btmanifest';
+              return '123_main';
             },
             nodeName: 'SCRIPT',
             nodeType: 1,
@@ -169,13 +169,13 @@ describe('contentUtils', () => {
                 nodeName: 'script',
                 nodeType: 1,
                 getAttribute: () => {
-                  return 'data-btmanifest';
+                  return '123_main';
                 },
                 tagName: 'tagName',
               },
               {
                 getAttribute: () => {
-                  return 'data-btmanifest';
+                  return '123_longtail';
                 },
                 nodeName: 'script',
                 nodeType: 1,
@@ -207,7 +207,7 @@ describe('contentUtils', () => {
       jest.resetModules();
       document.body.innerHTML =
         '<div>' +
-        '  <script data-btmanifest="123" src="https://facebook.com/"></script>' +
+        '  <script data-btmanifest="123_main" src="https://facebook.com/"></script>' +
         '</div>';
       scanForScripts();
       expect(window.chrome.runtime.sendMessage.mock.calls.length).toBe(1);

diff --git a/src/js/contentUtils.ts b/src/js/contentUtils.ts
@@ -226,9 +226,9 @@ function handleScriptNode(scriptNode: HTMLScriptElement): void {
   // versions, so we can use the first manifest version as the script version.
   const version = manifest1.split('_')[0];
 
-  if (!version) {
+  if (!version || !otherType) {
     invalidateAndThrow(
-      `Unable to parse a valid version from the data-btmanifest property of ${scriptNode.src}`,
+      `Missing manifest version or type from the data-btmanifest property of ${scriptNode.src}`,
     );
   }