search

Rothera / bpm

BetterPonymotes
GNU Affero General Public License v3.0
22 stars 23 forks source link

"Expando is not yet ready" - BPM shows RES warning about expandos. #41

Closed Two-Tone closed 6 years ago

Two-Tone commented 7 years ago

Users with RES and BPM will see this for any link that RES makes an expando for. Example image It appears to be a result of BPM loading in the middle of RES loading.

I can think of three ways to fix this: Blacklist the text so it doesn't appear, delay the creation of the link hover text, or have an (enabled by default?) option that hides link hover text but not emote text.

Rothera commented 7 years ago

I have never actually seen this for some reason.

Blacklisting special RES elements from title text expansion isn't objectionable; BPM has several special cases for RES. The ideal case is that the element has some easily detected class.

I rather don't want to break title text on links in any way. I find it handy, since redditors in the wild will make use of it on rare occasion, and RES doesn't do this itself.

Ajedi32 commented 7 years ago

FYI, I've noticed this as well. I had no idea it was an interaction between BPM and RES though. The problem appears intermittent for me; sometimes the text will appear and sometimes it won't.

Any idea what the actual root cause of this is? Is BPM displaying the title text for regular links? I thought it only did that for emotes. Is BPM somehow incorrectly detecting expando links as an emote?

Shugabuga commented 7 years ago

I think I found the object causing the issue. I believe it is the expando button itself, specifically:

<a class="expando-button toggleImage image collapsed collapsedExpando commentImg" title="" data-bpm_state="a" target="_blank" rel="noreferrer"></a>

Note the data-bpm_state. That usually only shows up if BPM does something with that object. I think making the code ignore expando-button classes should do the trick.

Rothera commented 7 years ago

FWIW yes, BPM expands alt text indiscriminately.