search

kheina-com / Blue-Blocker

Blocks all Twitter Blue verified users on twitter.com
Mozilla Public License 2.0
321 stars 28 forks source link

Old Twitter Layout - Problems with popups #260

Open kvba0000 opened 2 months ago

kvba0000 commented 2 months ago

I noticed an odd problem with the extension - more specifically when I use the other extension which recently got support "Old Twitter Layout" and extension blocks somebody it doesn't show popup about it. They only show up when I go to new twitter mode. Tried on both normal and extension compatibility mode.

From my findings I noticed this particular error when extension tries to show the popup. image

Both extensions are on the newest version:

Browser: Firefox Fork "Merkury" 123.0.1

rougetimelord commented 2 months ago

Blue blocker adds the toast element to body as soon as the content script loads. https://github.com/kheina-com/Blue-Blocker/blob/e84da3564a883ae0aa86eb142edbb150134c6343/src/content/startup.ts#L18-L20 I'm not sure how OldTwitter works, but I would suspect that it overwrites the body and destroys the toasts element. I think there are two possible solutions:

  1. Create a new toasts element if the old one gets destroyed
  2. Coordinate with @dimdenGD to make sure that the toast element doesn't get destroyed, or to communicate that OldTwitter is done modifying the DOM so we can inject the toasts element again
rougetimelord commented 2 months ago

I created an experimental fix branch on my personal fork. If you feel up to it you can test it out locally by:

  1. Run git clone --single-branch -b exp-260-fix git@github.com:rougetimelord/Blue-Blocker.git
  2. Run npm run build
  3. Run make firefox
  4. Visit the firefox addon debugging page
    • (or enter about:debugging#/runtime/this-firefox in the Firefox url bar)
  5. Click Load Temporary Add-on in the top right and select manifest.json in the blue-blocker/build folder

Let me know if that fixes the issue and I'll PR it into main!

kvba0000 commented 2 months ago

So I installed modules with npm i and then tried to build it. First issue I got was:

src/utilities.ts:150:35 - error TS18047: 'ele' is possibly 'null'.

150  const timeout = setTimeout(() => ele.removeChild(t), popupTimer);
                                      ~~~

src/utilities.ts:152:3 - error TS18047: 'ele' is possibly 'null'.

152   ele.removeChild(t);
      ~~~

which I simply fixed by adding ele && in those lines.

After building it and adding to extensions I got this error in extension's dev tools making extension unusable: image image

rougetimelord commented 2 months ago

I pushed a new commit that should fix the typescript errors, no idea what the lexical error is about though

kvba0000 commented 2 months ago

I tried to also load it in Brave with npm run dev and that's the error. Might be related to the one above image

rougetimelord commented 2 months ago

Huh weird! That's not related to any of the changes I made, give me a bit to try some things out

rougetimelord commented 2 months ago

Ok new changes are up, it doesn't have the RefID error anymore, and should (fingers crossed) address the issue more directly

kvba0000 commented 2 months ago

After pulling new update I noticed no RefId error on Brave Back to the popup issue: Now the error is gone but the popup is still not showing up. Weird...

rougetimelord commented 2 months ago

Well I guess that settles what the incompatibility is. OldTwitter is blocking the injection of new DOM elements into the document.

Not much that we can do on our end to fix that, probably worth opening an issue over on the OldTwitter repo. For your reference, the toast container has its id set to injected-blue-block-toasts. It should be pretty easy to check if an element getting added has that id, and let it go through.

dimdenGD commented 2 months ago

OldTwitter doesn't block creating new elements into the document

rougetimelord commented 2 months ago

Hmm, weird. No clue then

kheina commented 2 months ago

the error occurs due to blue blocker starting up quicker- bb adds the pop-ups div to the dom. later, as part of it's startup, old Twitter clears the dom. the correct solution is simply to re-add the pop-ups div to the dom if it's missing

rougetimelord commented 2 months ago

the error occurs due to blue blocker starting up quicker- bb adds the pop-ups div to the dom. later, as part of it's startup, old Twitter clears the dom. the correct solution is simply to re-add the pop-ups div to the dom if it's missing

On my experimental fix branch I added a function that either grabs the div or creates and appends a new one, so execution order shouldn't matter?? https://github.com/rougetimelord/Blue-Blocker/blob/exp-260-fix/src/utilities.ts#L113C1-L121C2