ui: Initialize color picker with user's color

[orblivion]

Initialize the color picker with user's current color. Was previously initialized with grey in farbtastic library. Fixes #3617

[JohnMcLear]

Awesome, any chance you could also add some test coverage? See https://github.com/ether/etherpad-lite/tree/develop/tests/frontend/specs for other test specs

Run tests by visisting /tests/frontend

Cheers :D

[orblivion]

I'm having an interesting time re-running tests after making changes to the files. I try restarting the server, restarting my browser, etc. It stays blank a lot of the time. Any advice on getting this to be consistent?

[JohnMcLear]

Do you have minify to false, maxAge to 0 and running etherpad in development mode (it will show on startup)?

[orblivion]

I've been in dev mode. I changed those two settings and it seems to be working. thank you!

[orblivion]

I guess it's not helping consistently.

[orblivion]

It seems to correlate to whether I currently have the file open in vim.

[orblivion]

I went off of the user name test and decided to add basic coverage for the color since I didn't see it anywhere. It includes coverage for my fix here.

I should note - it seems that the database doesn't reset per test runs, and the same goes for the user name test. This easily leads to a false positive for passing. (Though, I did delete the other test files while I was testing to speed it up. Maybe there's a database reset directive in one of those files?)

[JohnMcLear]

It makes sense for the database to keep things persistent, but yeah, I assume the author value is being reassigned, I'm not sure of a method to get a "new" author, perhaps faking some browser property or deleting some session / cookie on the client side? It's easy to programmatically delete cookies, did you try that?

[orblivion]

So, there's a clearCookies function in helper.js, and it's called by default as part of newPad (there's an option to not call it). However clearCookies does not work as named. Firstly, it seems you have to set each cookie with a past expiration date (as you do correctly elsewhere within these tests). Secondly, for some reason the cookie clearing doesn't seem to take effect in due time within newPad. It seems to need a delay in there; even a timeout of 1 seems to do the trick.

I made a WIP to fix both of these. Now it works as expected. Except now it breaks a bunch of other tests. Do you have any thoughts on what to try?

https://github.com/ether/etherpad-lite/compare/develop...orblivion:fix-clear-cookies-tests?w=1

My "false positive" concern here and in the name change test would be fixed by fixing the helper as described above. At this point it seems to be orthogonal to this PR. Could we make that a separate fix and move on with this PR?

[JohnMcLear]

I doubt @muxator would be happy to see MORE setTimeouts land in the source :D

I'm not sure cookies aren't cleared "before" newPad though, you would have thought that would be the case. But maybe it's supposed to be

Do a test, test cookies etc. Clear cookies.. Either way window.document.cookie = "" is wrong and your fix is better. I guess one solution would be to make clearCookies async or do a callback IE clearCookies(function(){if(!padname.... or await clearCookies()...

Can you keep looking into it today? I have to work on this other problem and I'm starting super late today ;\

[JohnMcLear]

Okay wtf. I can replicate your testing issue and it's horrible. I'm gonna debug that for you. Good find!

My findings. [using dirtydb]

1. Stopping Etherpad
2. Deleting var/
3. Starting Etherpad

Doesn't help

Clearing browser cache doesn't help

This is bonkers. I have no idea what's happening.. Gimme some time..

Eh.... After I edited the file it was saving but then it was back to it's original format when I went into the folder??? I have a feeling it's to do with pwd-esque related issues where the file system is changing and it think it's till in a stable state.. FWIW I'm using nano

[JohnMcLear]

Closer. The spec framework for some reason is using the .swp file for the specs... Fixing, assuming I can.

[JohnMcLear]

Fixed in #3745

Feel free to pull that and test. I'm gonna go back to working on fixing broken tests now.

[orblivion]

To be clear - what you replicated was related to editing with vim? Not the cookie issue?

[orblivion]

Confirmed that merging in #3745 fixes the vim problem 👍

[orblivion]

Strange, I wasn't able to replicate my own problem with the cookies that supposedly required the timeout. But, I wrote a test that would catch it if it were to ever happen again, and I put in the other part of the cookie fix, and the result is now in my working branch for this issue:

develop...orblivion:fix-clear-cookies-tests

After confirming that I didn't break any more tests, I'll make a new PR for that.

[JohnMcLear]

The new tests should be merged into develop today so just continue on this tomorrow with a pull of ether/develop into your branch.

[orblivion]

Sounds good. I ended up trying the existing tests already and it seems to not break anything new. Interestingly none of the importexport.js tests seem to pass for me irrespective of my change.

[orblivion]

(Firefox 68 esr on Debian on Qubes)

[JohnMcLear]

Interesting RE importexport tests not working. Same in Chrome/Edge et al? What database backend? Does import/export work in the UI? Thanks!

@muxator btw this can be reviewed/merged.. The conversation has switched to something else

[muxator]

The peculiar caching behaviour from Etherpad hit me when testing, too. I had to try three times. 😄

Pulled in, thanks.