Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

target.querySelectorAll is not a function #862

Closed
judos opened this issue Sep 8, 2022 · 9 comments · Fixed by #871
Closed

target.querySelectorAll is not a function #862

judos opened this issue Sep 8, 2022 · 9 comments · Fixed by #871

Comments

@judos
Copy link

judos commented Sep 8, 2022

  • Facebook Container Version: 2.3.3
  • Operating System + Version: Macos Monterey 12.5.1
  • Firefox Version: 104.0.2
  • Other installed Add-ons + Version + Enabled/Disabled-Status:

Add-ons
Name Typ Version Aktiviert ID
1Password – Passwort-Manager extension 2.3.7 true {d634138d-c276-4fc8-924b-40a0ea21d284}
Adblock Plus - kostenloser Adblocker extension 3.14.2 true {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}
Add-ons Search Detection extension 2.0.0 true addons-search-detection@mozilla.com
Amazon.de extension 1.9 true amazon@search.mozilla.org
Bing extension 1.3 true bing@search.mozilla.org
devedis Time extension 1.0.3 true hosting@devedis.ch
DuckDuckGo extension 1.1 true ddg@search.mozilla.org
eBay extension 1.3 true ebay@search.mozilla.org
Ecosia extension 1.1 true ecosia@search.mozilla.org
Everhour — Time Tracking, Budgets, Expenses extension 1.6.148 true time-tracker-on-site@everhour.com
Facebook Container extension 2.3.3 true @contain-facebook
Form History Control (II) extension 2.5.6.1 true formhistory@yahoo.com
Google extension 1.2 true google@search.mozilla.org
LEO Eng-Deu extension 1.0 true leo_ende_de@search.mozilla.org
Return YouTube Dislike extension 3.0.0.5 true {762f9885-5a13-4abd-9c77-433dcd38b8fd}
Wikipedia (de) extension 1.1 true wikipedia@search.mozilla.org
YouTube NonStop extension 0.9.1 true {0d7cafdd-501c-49ca-8ebb-e3341caaa55e}
Augury extension 1.24.1 false augury@rangle.io
DuckDuckGo Privacy Essentials extension 2022.8.25 false jid1-ZAdIEUB7XOzOJw@jetpack
Firefox Relay extension 2.4.5 false private-relay@firefox.com
OneTab extension 1.59 false extension@one-tab.com
Shortkeys (Custom Keyboard Shortcuts) extension 4.0.2 false Shortkeys@Shortkeys.com

Actual behavior

Developing on an angular application throws me the following error in the console:

TypeError: target.querySelectorAll is not a function6 content_script.js:694:27

Looks like after disabling the contain-facebook extension the error is gone.

Expected behavior

No exceptions in the content_script

Steps to reproduce

Install the same extensions I have
Open a small angular application (can't share the one i'm working on sorry)
Click around in navigation
Errors occur each navigation. Some form pages I have produce 5/6 of the same error.
On history pages with few data entries I get 2/3 of this error.

Notes

@mchurichi
Copy link

Same issue here, this is breaking even this same github issue page (you'll notice some missing css styles)

@phildrip
Copy link

phildrip commented Sep 8, 2022

This also causes problems with dark mode, on many sites, including github and MDN:
image

image

@jthieling
Copy link

  • Facebook Container Version: 2.3.3
  • Operating System + Version: Windows 10 Pro 19043.1889
  • Firefox Version: 104.0.2 (64-bit
  • Other installed Add-ons + Version + Enabled/Disabled-Status:

Same issue.

@bakulf
Copy link
Collaborator

bakulf commented Sep 8, 2022

Version 2.3.3 was buggy and, as a temporary fix, we just released a v2.3.4 as a v2.4.2 repackaged. Please, update the add-on to have a fix for this issue.
All the features introduced by v2.3.3 will be released in v2.3.5 as soon as possible.

@bakulf bakulf closed this as completed Sep 8, 2022
@groovecoder
Copy link
Member

Thanks everyone for the quick feedback on this. We will be more diligent about testing website compatibility in the future.

90K users received the buggy 2.3.3 version, and now 109K users are on the latest 2.3.4 (i.e., the rollback) version. 1.1M users are still on 2.3.2, but their next update should jump them straight to 2.3.4 instead of the buggy 2.3.3.

We are working on a full fix to include the security improvements, performance improvements, AND the bug-fix for the styling issue.

But this particular issue target.querySelectorAll is not a function is still happening in the latest branch with the styling fixes. 😢

@mrwensveen - can you help us debug this issue? It seems to have been introduced by #736.

@judos
Copy link
Author

judos commented Sep 9, 2022

Thanks for the fast release! I'm now on 2.3.4 and it looks good. Hope you can get it fixed on the latest branch as well 👍

@mrwensveen
Copy link
Contributor

I'll take a look at it this weekend. Sorry if my fix caused any inconvenience.

@mrwensveen
Copy link
Contributor

I think I know what the issue is, but I can't reproduce it myself. Is there a clear way to reproduce this issue so I can test my fix?

@mrwensveen
Copy link
Contributor

mrwensveen commented Sep 10, 2022

Steps to reproduce on any page (that contains at least one link):

  1. Open the console in devTools
  2. let a = document.querySelectorAll('a')[0];
  3. a.innerHTML = 'test';

This creates a new text node. The mutation observer sees this and calls the callback with mutation.type === 'childList'. mutation.addedNodes are then sent to contentScriptInit as the target parameter. However, text nodes do not have a querySelectorAll function.

I think the easiest way to fix this is to filter the list of added nodes on node.nodeType === Node.ELEMENT but we could also test for typeof target.querySelectorAll === 'function' at the call site. The latter has the advantage that Document and DocumentFragment nodes detected by the mutation observer are also processed, but I highly doubt it is even possible to add a Document or DocumentFragment node to a normal element. Testing nodeType is clearer and more performant.

I'll send a PR if you decide that that's what you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants