search

jesus2099 / konami-command

power‐ups for various web sites
122 stars 25 forks source link

Still visible in the sidebar but nothing appears in the tracklist (ISRC, comments, AcoustID) any more #596

Closed jesus2099 closed 1 year ago

jesus2099 commented 3 years ago

↖ #38 – NG with release with more than 10 mediums after MBS-3841

Workaround is to make this script near or at the end of your userscript list.

We cannot see recording comments since some change on beta, that will probably occur on MBS to, soon.

Spotted by @Lotheric: https://github.com/jesus2099/konami-command/issues/445#issuecomment-864572633

Related to

jesus2099 commented 3 years ago

There is still problems with the other tools (recording open edits, relate/merge tools and Edit rec. buttons). It seems to be because of react-hydrate nightmare redrawing the last column. 😨 But beta release table last 2 columns are not stable yet, see how ratings are not clickable: https://tickets.metabrainz.org/browse/MBS-11735

tigerman325 commented 3 years ago

Mass Merge Recordings & Set Comments for Recordings for a release have just stopped working for me. They show that they are loaded, but the button for Set Comments for Recordings no longer shows up and when I add a release into the field for remote release box, it never loads them.

jesus2099 commented 3 years ago

It must be because of https://tickets.metabrainz.org/browse/MBS-11703 that may have introduced react-hydrate (page redraw) stuff.

tigerman325 commented 3 years ago

The release comment script is back working. So at least that was short lived.

jesus2099 commented 3 years ago

The release comment script was not updated in 10 month but will hopefully be fixed by its author @mwiencek soon. ;) As it's himself, bitmap, who changed release page recently in MBS-11703.

Maybe it works for you because it is running enough late (behind lots of other scripts), after react has done its deferred page redraw, or something.

As I don't understand react, I will be able to learn from his fix. I am in search of a clean stable reliable solution for these react-hydrate-deferred-page-redraws rather than my delays and other tricks.

jesus2099 commented 3 years ago

In fact I found the react hydrate stuff in:

mwiencek commented 3 years ago

Sorry this causes so much trouble. :\ The main issue I see is that React will remove any elements you inject whenever it re-renders the parent. I'm not sure of the best way to handle this. MutationObserver could work somehow, but might be slow and tedious.

I could also add some callbacks in the MBS components that userscripts could hook into and modify stuff, though this probably still requires some minor knowledge of React. It might be the most reliable option though.

Could you make a list of the different elements on the release page you need to modify/inject into?

jesus2099 commented 3 years ago

Thanks for chiming in, @mwiencek. :)

The main issue I see is that React will remove any elements you inject whenever it re-renders the parent.

Exatcly!

I thought there were some React functions that I could call to tell React this and this had to be kept at next page redraw, There is no such thing as a kind of react.register(myElements) function?

I don't think MBS code should go as far as to include specific callbacks for user scripts, though. It would be too much.

If there is no clean react way, I try to make some clean new code based on setInterval or setTimeout functions, like I do for more and more scripts (GitHub user scripts required that, the first). I remember I tried MutationObserver but there were some problems I don't remember. I only remember it was simpler and more reliable to use dumb setInterval or setTimeout functions.

mwiencek commented 3 years ago

I thought there were some React functions that I could call to tell React this and this had to be kept at next page redraw, There is no such thing as a kind of react.register(myElements) function?

Sadly no. setInterval is probably your best bet, then -- if the elements you insert have unique IDs, getElementById will be a very fast way to check if they were removed, allowing a smaller interval.

jesus2099 commented 3 years ago

Thanks! 😁

jesus2099 commented 3 years ago

Hi @mwiencek, did the div.loading-message disappear from loading expand/collapse mediums? It seems there is now only:

<tr><td colspan="4" style="padding: 1em;">Loading...</td></tr>

Which makes it difficult with multilingual MBS.

Before that, I used to have tr > td > div.loading-message.

Update

Indeed:

Before

https://github.com/metabrainz/musicbrainz-server/commit/34fbf7d6e9793e13ac54d5ec82992f3339d47011#diff-b4fa79da081c94745153af98e38b774364465df255d3dccd9e254b8ec302e11aL60-L63

<tr>
    <td>
        <div class="loading-message">
            {l('Loading...')}
        </div>
    </td>
</tr>

After

https://github.com/metabrainz/musicbrainz-server/commit/34fbf7d6e9793e13ac54d5ec82992f3339d47011#diff-a94cec52e8d4657c88b7e1066cd4276083e045038928cf5c871611392d94fe34R315

<tr>
    <td>
        {l('Load all tracks...')}
    </td>
</tr>

Do you think this kind of stuff could be put back?

reosarevok commented 3 years ago

I'm pretty sure it could. I'll look into making the change.

reosarevok commented 3 years ago

https://github.com/metabrainz/musicbrainz-server/pull/2166 - does that look like it would work? (it'd need a small change at least since there's no longer a div, but)

jesus2099 commented 3 years ago

Good release example

To test this out: https://musicbrainz.org/release/16caa039-aba9-45e8-9ff1-8cca8635ff52

jesus2099 commented 3 years ago

Work amount is also broken, I now see only Tracks: https://musicbrainz.org/release/5ebe09a7-2451-47aa-91eb-9a9ce5973ef3

jesus2099 commented 2 years ago

I will fix this by finally writing a new version that handles all kinds paginated releases (#38), like recently done for COLLECTION HIGHLIGHTER.

It will solve this issue at the same time!

jesus2099 commented 2 years ago

This bug will soon trigger all the time, like on beta: see examples in #682.

redactedscribe commented 2 years ago

This bug will soon trigger all the time, like on beta: see examples in #682.

Now triggering on non-beta site too.

ROpdebee commented 2 years ago

@jesus2099 wrapping your INLINE STUFF script into a window load event handler seems to be a viable workaround on my end.

window.addEventListener('load', function() {
  // rest of the script
});

I don't think it's truly a reliable workaround, but the React hydration errors disappeared and INLINE STUFF seems to do its job. Alternatively, @run-at document-idle seems to solve it as well.

The reason it's not a reliable workaround is that it's also waiting for all images to finish loading. So, if the Internet Archive/CAA is having a particularly bad day, this workaround could make it so your script will only execute after the sidebar cover art is loaded, which is of course not ideal.

MutationObserver, as recommended by bitmap previously, won't work for INLINE STUFF's hydration errors (nor will it work for my inline track recordings script, https://github.com/ROpdebee/mb-userscripts/issues/484) since there's no client-side rendering going on (unless you trigger the hydration errors, in which case React will fall back to client-side rendering).

What I think could be a decent solution would be a custom event that gets dispatched whenever hydration of a component has finished. Userscripts could then listen for that event and postpone their modifications until it arrives. Ideally the event would bubble and be dispatched on the hydrated component's root, so we can listen to it anywhere in the parent chain but still figure out which element exactly was hydrated. That also wouldn't require any tricky hooks made specifically for userscripts, and no deep React knowledge required for the userscript authors. Similar events dispatched in the client-side rendering hooks (componentDidMount and componentDidUpdate) would be even more amazing, then we could get notified of when userscript elements need to be reinserted instead of having to observe for that. @mwiencek does this sound feasible to you (primarily the hydration part)?

jesus2099 commented 2 years ago

Thank you very much. Anyway I will surely do like I did for COLLECTION HIGHLIGHTER (setInterval: https://github.com/jesus2099/konami-command/commit/463183e3565fccb3aeffb8cac7f0647aa21e0f74). I need to support all tracks, even those that are loaded later, with all kinds of collapsed/paginated releases.

But it might be good to put most of my scripts in document-idle.

But at the moment I am on several other stuff, that I need more. And the workaround works for me. Maybe I will do a quick fix as you suggest, for others.

jesus2099 commented 2 years ago

In my past https://github.com/jesus2099/konami-command/labels/react-hydrate 12 fixes "researches," I think it was not possible to wish for such post-hydrate events. I think I always came to the conclusion that setInterval was actually the best solution, and not so dirty, after all.

jesus2099 commented 2 years ago

@redactedscribe @Alfge, tell me if my quick fix did the trick for you.

a23bed commented 2 years ago

last update works fine now. Thanks

redactedscribe commented 2 years ago

tell me if my quick fix did the trick for you.

Yep, works, thanks (on beta too)!

Alfge commented 2 years ago

Tried the "Workaround is to make this script near or at the end of your userscript list." but doesn't help for me :-(

jesus2099 commented 2 years ago

@Alfge did you update to version 2022.6.14, already? This is the version with document-idle quick-fix, suggested by @ROpdebee, working for most people.

mwiencek commented 2 years ago

@mwiencek does this sound feasible to you (primarily the hydration part)?

Yes, I think custom events is a good idea.

Alfge commented 2 years ago

Just updated to version 2022.6.14 and now it works for me too. Thank you!

jesus2099 commented 1 year ago

https://musicbrainz.org/release/16caa039-aba9-45e8-9ff1-8cca8635ff52

For this release, still no comments, no ISRC, no AcoustID.

jesus2099 commented 1 year ago

Today, that release recording comments show up (on mobile). It's very random until I fix.

jesus2099 commented 1 year ago

I added that same old lame react hydrate workaround (delay): https://github.com/jesus2099/konami-command/commit/e54015618b4bc78899e29569ad08575bb7ff34dd?w=1