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

"People and body" section doesn't work inside Firefox addon content script #356

Closed
dimdenGD opened this issue Jun 29, 2023 · 10 comments · Fixed by #357
Closed

"People and body" section doesn't work inside Firefox addon content script #356

dimdenGD opened this issue Jun 29, 2023 · 10 comments · Fixed by #357
Labels
bug Something isn't working

Comments

@dimdenGD
Copy link
Contributor

When ran in Firefox addon's content script, opening "People and body" section doesnt work, it doesn't render emojis inside even though it switches the active section.
Error: Error: Not allowed to define cross-origin object as property on [Object] or [Array] XrayWrapper inside cleanEmoji function, specifically emoji.skins = Array(len);, seems that assigning to emoji causes this error but I wasn't able to figure out what's exactly wrong. Something about Firefox's sandbox

Related issue: dimdenGD/OldTwitter#77

@nolanlawson nolanlawson added the bug Something isn't working label Jun 30, 2023
@nolanlawson
Copy link
Owner

@dimdenGD Thanks for filing! Do you have a test to reproduce? Have you filed a bug on Firefox?

I've only glanced at this issue, but it seems unlikely that it can be fixed on the emoji-picker-element side of things... It appears to be a Firefox issue.

@dimdenGD
Copy link
Contributor Author

There seems to be almost identical bug filed: https://bugzilla.mozilla.org/show_bug.cgi?id=1475950 which was closed already. I don't think this a Firefox bug, just a specific security measure on Firefox that needs to be addressed properly when detected to be used inside Firefox sandboxed window. The only (easy) way to reproduce right now is to use my extension Old Twitter Layout on Firefox and open emoji picker in it.

@nolanlawson
Copy link
Owner

This definitely appears to be a bug in Firefox. I believe the error is caused by the line:

Based on the bug, it looks like Firefox is doing some kind of sandboxing of globals; I assume the global Array is confusing it here.

I don't know the first thing about Firefox extension development, so I can repro but I don't know how to test a fix. In your use of the emoji picker, could you try:

-          emoji.skins = Array(len);
+          emoji.skins = [];
           for (let i = 0; i < len; i++) {
-            emoji.skins[i] = {
+            emoji.skins.push({
               tone: emoji.skinTones[i],
               unicode: emoji.skinUnicodes[i],
               version: emoji.skinVersions[i]
-            };
+            });
           }

If that fixes the issue (and there are no other bugs!) then you have a repro you can file on Mozilla for sure.

@nolanlawson
Copy link
Owner

My theory is that what's happening here is

  1. The object comes back from IndexedDB
  2. We try to set Array() on it
  3. Firefox is confused about the origin of the IndexedDB object versus the Array global
  4. Boom

Just a theory, though.

@dimdenGD
Copy link
Contributor Author

Originally I thought it's caused by assigning anything into emoji but I tested and assigning a number works fine, before opening this issue I also tried basically same code as you just showed and it didn't work, so neither Array nor [] works for this.
Using window.wrappedJSObject.Array() seems to work for first line, but then it errors on version with same error for whatever reason:
image

@nolanlawson
Copy link
Owner

Could you try this:

      function cleanEmoji (emoji) {
        if (!emoji) {
          return emoji
        }
+       emoji = structuredClone(emoji);
        delete emoji.tokens;

Maybe cloning it will cause Firefox to stop complaining? If structuredClone doesn't work, you can also try JSON.parse(JSON.stringify(emoji)).

@dimdenGD
Copy link
Contributor Author

That fixed it! and no window.wrappedJSObject needed to work too

@nolanlawson
Copy link
Owner

Thanks! FWIW I noticed that the skintone picker also appeared to be not working in your extension. (Open the dropdown, select a skintone, it never updates.) Not sure if that's related.

@dimdenGD
Copy link
Contributor Author

Just checked, it seems to update the emojis, but only after going to other category and then back to People and body. Not sure if it's supposed to update hand skin color itself. This happens on both Chrome and Firefox and doesn't throw any error. I'll check this out

@dimdenGD
Copy link
Contributor Author

Apparently it's because of Twemoji use, so it's unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants