calzoneman / sync

Node.JS Server and JavaScript/HTML Client for synchronizing online media
Other
1.47k stars 232 forks source link

Editing layout with custom javascript; broken by ChannelCSSJS/setMotd call order #394

Closed dany-on-demand closed 10 years ago

dany-on-demand commented 10 years ago
I wanted to add support for different languages in the MOTD, and decided to use bootstrap tabs(pills) to do so, IMO easiest and fastest solution, it's only a few lines of text anyway.

Well apparently this needs some JS back code, so I added it, great it worked, but wait no, not after you reload the page.

So it turns out the custom javascript gets executed before the MOTD is set, not sure if this is intentional( security concerns:question: :warning: ) but doesn't that almost completely ruin the idea why it was added in the first place? Well, so when I set the JS after loading the page it works because it's refreshed after MOTD html is there.

Another thing I wanted to do was to add a warning "Please allow javascript in this channel to support dynamic content" and remove it via JS (obviously). That doesn't work too of course.

I could "hack" it, just execute the JS after a timer, but that's just ugly as hell. I like things proper if possible.

I tried actually hacking it, I thought if I can't get to the HTML this way, maybe I can just set the HTML in the JS code, it's short it'll fit the 20kb limit. But I didn't realize that setMotd will just clear anything there was if the MOTD custom data is empty. Damn. And the JS warning thing wouldn't appear anyway, what a waste :/

Well, any suggestions:question: The channel is at http://cytu.be/r/ex-yu_movie_club

~Sleepy Dany

calzoneman commented 10 years ago

Your code should not depend on the order in which messages are received.

Custom JS is not guaranteed to be executed before/after the MOTD is set

The ordering of CSS and MOTD is arbitrary, but I don't think explicitly lining up my packet order to suit specific channels' needs is a sustainable option.

I think the best way to handle this is to add your own listeners to the events CyTube sends. For example:

socket.on("setMotd", function (data) {
    // The MOTD has been updated, do something to it.
});

You could easily wrap this in a helper function that checks if the desired state is present, and if it isn't, adds a listener for the event. Let me know if you have questions.

dany-on-demand commented 10 years ago

Thanks a lot!I Think I fixed it alright Just a few more notes... I just accidentally lost all of them so here's a lazy summary:

  1. dismissable btstrap alerts don't work because data-dismissable is truncated by html sanitizer and blows it, had to hack around this
  2. tabs/pills dropdown menu bug, actually now that I think about it I think it was me who broke it, I don't know
  3. way too much random "< br >" glitch/bug; bootstrap 3.0.3 thing maybe?
  4. [suggestion] make the polling an external service or something, I want to embed a continuous poll
  5. The HD layout isn't really HD. But I like it anyway, it should be the default one IMO.

~Even sleepier dany

calzoneman commented 10 years ago
  1. I'm not sure why the sanitizer is catching data attributes. I can look into it.
  2. ?
  3. For legacy reasons, the MOTD sanitizer replaces newlines with <br>. It might be worth running a little script to update existing MOTDs and then remove this behavior, I'll have to think about it.
  4. I'm not sure what you mean by this. The internal polling system is intended for short term polls (e.g. voting on the next video). If you're wishing to conduct a long-term survey, there are plenty of other websites that allow you to do so.
  5. The name "HD" for that layout is once again an artifact from the past, but I agree it's not really descriptive. If you have a better name I'd love to hear it. The reason it is not default is that it doesn't fit on small displays. The default layout was picked so that it fits comfortably on my laptop's 1360x768 display.
dany-on-demand commented 10 years ago

edit: also 1. > it takes data-dismissable makes it datadismissable removes the -

.2. adding a dropdown pill/tab, if you click on it the dropdown menu stays on forever no matter what, I think I might've caused it though by calling the function that enables the dropdown menu twice .5. no idea, I will now use the sleeping pod of magical time teleportation to cure my tired brain

calzoneman commented 10 years ago

Marking as closed unless there are further issues.