Running Promnesia crashes Facebook (and some other sites)

[Stvad]

Recipe is something like:

• Go to FB page that has Promnesia matches
• After promnesia loads - scroll timeline
• 💥

---

A similar story occurs on my digital garden website: https://vlad.roam.garden. Algorithm is similar but instead of scrolling timeline - the action is to open internal ink (which should open in a new side-panel).

The common thing between those two, I think is, triggering a React re-render

Running it in Firefox 109.0b6, macOS 13.1

[karlicoss]

Yeah, can confirm it happens on your garden site.
I think the issue is caused by "mark as visited" feature (it says 'experimental' because I sort of anticipated it might break stuff). Can you confirm it doesn't crash when "mark as visited" is disabled?

To workaround there are a few options at the moment

• disable Promnesia on "vlad.roam.garden" altogether (sad, but presumably on your own garden it's not as useful anyway since you remember more context)
• disable "mark as visited" by default and only use it via hotkey (a bit sad)
• use "exclude multiple links from mark as visited (element zapper)" in promneisa menu on the site and select the main column of your site -- at least it worked for me. Also somewhat sad option because it means links to your garden won't be marked when linked from other sites

I think the right thing to do (considering it's inevitable that page modifications would break something) would be:

• separate setting to disable "mark as visited" feature for a domain/regex/exact page. Exactly same semantics as normal blocklist for Promnesia, just a separate one. Possibly same for highlights feature as well?
• the zapper should block DOM path (similar to the way ad blockers work) rather than using the URLS inside that dom path

[Stvad]

Can confirm that disabling Mark visited prevents crashes both on roam.garden and FB.

Will think on what workaround works best 🤔

Proper solutions wise: I think it's possible to make the feature work more reliably without crashing things.

For example my extension https://github.com/transclude-me/extension does very similar things mechanics-wise (mark up relevant links & show popup when you hover over them) without causing any crashes (afaik)

I believe what makes the difference is that I do mark-up in CSS-only way (add a class to relevant links and style them differently) and don't modify the DOM

[karlicoss]

Interesting, thanks for the reference! (cool extension by the way :) )

IIRC the main reason I was wrapping elements into extra HTML was to add a bit which activates the popup on hover -- not sure if it's possible with CSS?

I guess in your case it works because you're using a hotkey + hover to activate -- I'd personally prefer if I didn't have to. Although maybe it's a good compromise in return for managing with CSS only (or could be configurable depending what the user preferes).

promnesia/extension/src/showvisited.js

Lines 186 to 199 in 32b8e9a

	* So, there are a few requirements from the marks we're trying to achieve here
	* - popups should be togglable (otherwise too spammy if shown by default)
	* - should show some visual hints that the popup is present at all (but shouldn't be too spammy)
	* - shouldn't change links placement in DOM -- that breaks many websites (and kind of annoying)
	* - shouldn't overlay any existing dom elements unless popup was clicked
	* - shouldn't break text selection
	* Returns extra elements to insert in DOM: (i.e. if they don't belong to any particular existing dom element)
	*/
	/*
	* Current implementation
	*
	* <a href... (element)> => <span (outer)><a href ... (element)> <toggler eye ... > <popup> </span>
	*/
	function showMark(element) {

[Stvad]

um, I think you can just add a onHover event listener to the link element?

In my case I'm outsourcing it to the Tippy.js library, the hotkey part is not something essential to it working, just a preferred UX for me.

[karlicoss]

Ha, I guess I was a bit too much in the "static html" mindset, didn't even occur to me to do it via javascript. I guess that makes more sense in this case considering the tradeoff of modifying the page, yeah.

Thanks for tippy.js -- would be very cool if I can replace my custom code with something mature 🙏

For my own reference later:

• background-image: linear-gradient(to right, transparent 80%, green 80% 100%); -- could be used on links to hint the hoverable area
• something like this to activate on hover: https://github.com/transclude-me/extension/blob/cae110bb97b5b4892b6dca5ca2d90ee83ecd644d/source/content/onboarding-tooltip/index.ts#L25
• looks like it has sticky plugin https://github.com/atomiks/tippyjs/blob/8f7b2df156ae39c4df23c481b77ab59fe7f08c44/website/src/pages/v5/all-props.mdx (implemented via javascript, but whatever)

I might give it a try soon -- I've been meaning to update extension towards better Manivest v3 support anyway.

If you think Promnesia could benefit from any other existing libraries -- please throw in in your suggestions :)

[Stvad]

looks like it has sticky plugin https://github.com/atomiks/tippyjs/blob/8f7b2df156ae39c4df23c481b77ab59fe7f08c44/website/src/pages/v5/all-props.mdx (implemented via javascript, but whatever)

fyi you're looking at the version of the docs for the older version of the lib https://atomiks.github.io/tippyjs/v6/all-props/#sticky

[Stvad]

I might give it a try soon -- I've been meaning to update extension towards better Manivest v3 support anyway.

🔥

If you think Promnesia could benefit from any other existing libraries -- please throw in in your suggestions :)

will do!

[Stvad]

something like this to activate on hover: https://github.com/transclude-me/extension/blob/cae110bb97b5b4892b6dca5ca2d90ee83ecd644d/source/content/onboarding-tooltip/index.ts#L25

also that is me specifically adding the "guard button" support bc tippy does not provide that by default. basic usage is a lot simpler: https://atomiks.github.io/tippyjs/#html-content

[Stvad]

https://github.com/GoogleChromeLabs/text-fragments-polyfill plausibly as a one way to do highlights

[Stvad]

I used https://github.com/caroso1222/notyf for notifications, though not sure how annoying is to do them yourself

[Stvad]

very likely not worth it to switch if things are working, but I find Parcel a lot more pleasent to work with vs WebPack

[karlicoss]

I used https://github.com/caroso1222/notyf for notifications, though not sure how annoying is to do them yourself

Thanks -- I actually used https://github.com/apvarun/toastify-js -- not sure which is better. For some reason it's vendorized in promnesia instead of being a proper npm dependency -- perhaps some webpack shenanigans.

I find Parcel a lot more pleasent to work with vs WebPack

thanks, also noticed you were using it and was surprised to find it had almost no configuration 😅 I might try it

[karlicoss]

some WIP on tippy.js, after a bit of fighting with webpack looks promising 👀

[karlicoss]

@Stvad curious, how did you manage to release your extension with Tippy.js without running in linting errors during publishing due to innerHTML assignments? atomiks/tippyjs#1041 (comment)

[karlicoss]

So, managed to release after all with some horrible JS patching, but it works!
For now just put a self-signed Firefox version + unpacked chrome version here https://github.com/karlicoss/promnesia/releases/tag/extension-wip-tippyjs , if you wanna test will appreciate it :)

Basically it shouldn't do any DOM structure modification for "mark visited", doesn't crash facebook for me 😎

Doesn't break https://vlad.roam.garden anymore, but seems like there is a style clash between tippies used on your site and the ones in Promnesia -- odd since I specifically used an extra CSS class to prevent this, but will investigate more before merging

[Stvad]

@Stvad curious, how did you manage to release your extension with Tippy.js without running in linting errors during publishing due to innerHTML assignments? atomiks/tippyjs#1041 (comment)

hrm, I haven't really used the web-ext lint before 😅. I suppose these particular warnings are not cause for review rejection 🤷‍♂️

[Stvad]

So, managed to release after all with some horrible JS patching, but it works! For now just put a self-signed Firefox version + unpacked chrome version here https://github.com/karlicoss/promnesia/releases/tag/extension-wip-tippyjs , if you wanna test will appreciate it :)

Basically it shouldn't do any DOM structure modification for "mark visited", doesn't crash facebook for me 😎

Doesn't break https://vlad.roam.garden anymore, but seems like there is a style clash between tippies used on your site and the ones in Promnesia -- odd since I specifically used an extra CSS class to prevent this, but will investigate more before merging

Yay! Will give it a try!

[Stvad]

First feedback is that it doesn't seem readable in Dark mode

(not sure if that's new or I never tried it in dark mode before 😅)

also seems to not quite be on top of all things here 🤔

[karlicoss]

Hmm, so the issue with tippy on https://vlad.roam.garden/ is that when promnesia injects tippy CSS into the page, it seems to override some attributes (even if I don't do anything else with the page)

Tippy has a namespace configuration, but it's a build-time thing, so not sure how to change it when it's already bundled up... https://github.com/atomiks/tippyjs/blob/master/.config/rollup.config.js#L12

Regarding dark mode -- tried dark mode on your site, but it looks okay, can you link the exact page where you seeing this? :)

[karlicoss]

Actually reproduced the dark mode issue -- and also fixed other glitches on your garden. The trick was to use headless tippy.js since it's not as intrusive (also solves the problem with innerHTML)
New pre-release: https://github.com/karlicoss/promnesia/releases/download/extension-wip-tippyjs/promnesia-1.2.0.6.xpi

[Stvad]

Nice! FWIW the dark mode issue was on FB

[Stvad]

Happens only when I've just installed the plugin, but thought I'd save it for posterity 😛 :

[karlicoss]

Ah, yes I've seen it haha -- generally things are glitchy on extension update -- perhaps need to support some sort of callback. But been low on my priority list since releases are not that often

[karlicoss]

Also noticed tippy isn't working on google search page

To be fair, old (current) version of tooltips isn't working well either -- even not showing the outline properly

have to use 'double click' logic to bring it on top -- so perhaps it's more of an issue with

[karlicoss]

Right, so after investigation it seems that it's because one of the parent elements for the link has contain: layout CSS property, which prevents stuff 'inside' of that parent elements to poke out of it. So the tooltip ends up clipped.

It seems that it might have been fixed in popper (which is renamed to floating-ui now?), but tippyjs is using a version which is too old. There is an issue for migration, but not sure if it happens anytime soon atomiks/tippyjs#1071

It's also possible to create tooltips in floating-ui directly, but I'm worried I'll have to reimplement quite a few things

Ended up doing a hack, that finds the closest parent element without contain: layout and attaching the popper to that. Although worried it might break something else...

[karlicoss]

damn.. turned out to be a bit easier, needed to use appendTo: document.body: https://github.com/atomiks/tippyjs/blob/master/MIGRATION_GUIDE.md#props-1
Seems that it might break accessibility, but I guess sadly promnesia isn't super accessible anyway, so we can solve it later, or make configurable via options https://atomiks.github.io/tippyjs/v5/accessibility/#interactivity

[karlicoss]

updated version https://github.com/karlicoss/promnesia/releases/download/extension-wip-tippyjs/promnesia-1.2.0.9.xpi

[Stvad]

a bit late for your various woes (though may still be useful as a general approach): in my extension I attach the tippy to a node in a shadow dom, which allows me to isolate it's CSS/etc. Generally putting your page elements into a shadow dom is very helpful to minimize interferance

[karlicoss]

Oh interesting! First time I've heard of shadow dom, will check it out. Do you know if there is a meaningful difference with iframe? E.g. would it make sense to put the sidebar in shadow dom instead?

[Stvad]

E.g. would it make sense to put the sidebar in shadow dom instead?

If you're happy with it - I probably won't bother. I think shadow dom allows for easier integration of the component into the main page, while preserving the isolation (e.g. matching layout, etc). But that's not really relevant for the sidebar (or a popup, I suppose) as it stands alone.

And in my model it's easier to programmatically interact with things across the shadow boundary.

[karlicoss]

I've been running the tippyjs version and think it's been very smooth -- so will merge soon and start motions to release in chrome/mozilla stores :)

[karlicoss]

It's released everywhere!

[Stvad]

this keeps happening consistently, actually 🙃