nolanlawson / emoji-picker-element

A lightweight emoji picker for the modern web
https://nolanlawson.github.io/emoji-picker-element/
Apache License 2.0
1.43k stars 82 forks source link

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

Closed dimdenGD closed 1 year ago

dimdenGD commented 1 year ago

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: https://github.com/dimdenGD/OldTwitter/issues/77

nolanlawson commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

https://github.com/nolanlawson/emoji-picker-element/blob/d4365af4423cc4ecfc967bfa414011039c1b1b36/src/database/utils/cleanEmoji.js#L10

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

nolanlawson commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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