search

sandstormports / community-project

Tracking our collaborative progress as a team
3 stars 1 forks source link

Dan: Upgrade Etherpad package (to 1.18.14 or close) #15

Closed orblivion closed 2 years ago

orblivion commented 2 years ago

Total

(This got a little complicated since there was wrapup stuff not really in any category. Will probably leave milestone metrics below as "out of date" unless people are very interested.)

Milestones (out of date)

This section got out of date as far as hours, and it'll probably take more time than it's worth to get back up to date. But keeping around in old state in case anyone cares.

Get started on spk based on Ian's vagrant-spk version

Caveat on vagrant-spk -> spk:

Based on Kenton's changes:

orblivion commented 2 years ago

Not sure how much community feedback you're looking for first, so lmk when I should go ahead.

I'd start with vagrant-spk -> spk, since testing everything else sort of depends on that.

ocdtrekkie commented 2 years ago

One of the challenges we've encountered through this process is soliciting community feedback. I think if Ian and I are good with it (I am), I think you should start working, as we are the two people currently administrating this funding, and there isn't a conflict of interest (i.e. we are not paying Ian). The terms we are discussing are not dissimilar from what we'd previously discussed if Ian took on the project, so there's been plenty of time for people to raise objections to the rate or the goal.

So perhaps wait for @zenhack to sign off here, and then consider that a go-ahead.

garrison commented 2 years ago

All sounds good to me. I am merely curious if someone could post a few sentences explaining the vagrant-spk -> spk change (motivation, etc.).

zenhack commented 2 years ago

I think the reason for this is that @orblivion uses QuebesOS, which, since it uses virtualization for its own isolation layer, makes running VMs difficult. I think if that initial overhead will make him otherwise able to be more productive it makes sense, though it is important that the build process is well documented and portable.

zenhack commented 2 years ago

Also, I think we should send out a thing to the mailing list and wait a day or two before pulling the trigger.

zenhack commented 2 years ago

(I sent out a thing)

orblivion commented 2 years ago

That's right, I can't really do a VM within a VM. And I just enjoy the simplicity of it. Qubes lets me make a VM to separate everything anyway, I may as well just have simple access to the machine.

That said, since I'll be paid for this, and since others may need to pick up the work after me some day, I'm thinking I should take a minute to look into running Vagrant on QubesOS. Maybe there's a way with some sort of caveat. Perhaps it could use Qubes' VM system, or perhaps it can work as totally emulated (and not unusably slow).

orblivion commented 2 years ago

Yeah, if there's a way it's not very obvious. It doesn't work out of the box at any rate.

zenhack commented 2 years ago

Alright, it's been few days and no one has spoken up; I think we can give you the green light.

orblivion commented 2 years ago

Works with spk in addition to vagrant-spk:

https://github.com/orblivion/etherpad-lite-sandstorm/

Turns out there's not a whole lot different between the two for this package so hopefully vagrant-spk still works just as well. See README.

Includes finishing up an optimization that Ian started (which Kenton didn't even include).

orblivion commented 2 years ago

Oh, big caveat: spk pack produces something where sandstorm can't seem to find any files referenced by the packagedef (???). @zenhack says he will look into it; it's only blocking release, not development. Worst case we can see if someone else can build it.

garrison commented 2 years ago

Great!

Oh, big caveat

Yeah, this sounds surmountable to me.

orblivion commented 2 years ago

Next is installing dependencies. See latest on my repo for the results of that. (4 hours so far.)

I initially included the altered plugins (ep_comments_page and ep_author_neat) in the list for the Dependencies milestone. This was a mistake given that they each have their own milestones.

Just for the sake of sticking to the estimate plan, I tried rolling with it installing them as-is, and they sort of looked promising, but there seem to be upstream changes that are required for the latest etherpad-lite. I'll need to rebase Kenton's changes for them onto latest upstream in their respective milestones.

Assuming people are fine with excluding the altered plugins from the Dependencies miletsone, there's one more thing before I call this done. I'd like some feedback on what I found with the unaltered plugins (see below).

Results

sqlite3

Ian already had taken care of sqlite3.

capnp

I had to wrestle a bit with capnp but I got it to the point where I'm able to require("capnp") without complaints. Confirming that it actually works is part of the "activities" milestone (in retrospect, estimating activities at .5 hours might have been a mistake given that capnp is a newer verison than our previous package, but I could get lucky).

unaltered plugins

They seemed to work fine with the below caveats. I'm curious about your opinions.

Note: Some are New Bugs (only after upgrade) and Existing Bugs (already exist in our previous package). Also, none of these bugs seem to be triggered by being within Sandstorm. They exist when running Etherpad directly.

ep_sticky_attributes

This plugin is supposed to make it so that if you click "bold" (etc) and type, it will type bold. Like a normal word processor. Without this plugin, the only function of the bold button is to format selected text.

Existing Bug: turning on bold, italics etc is indeed sticky, but turning it off is not. (I suppose you could say it's too sticky)

ep_font_family:

Font choice is not "sticky", and ep_sticky_attributes does not change this. You can only select areas and change their font. Though, if you move your caret to the middle of an area that is already a different font, then you can continue typing in that font.

New Bug: The font dropdown menu does not react when you move the caret to an area that no longer has that font.

Granted, in the previous Etherpad package, moving the caret would just switch to "Font Family" (the name of the dropdown) rather than telling you the name of the font at the new location of the caret. But at least it wouldn't be incorrect information.

ep_font_size:

New Bug: Same bug as ep_font_family but for font size.

ep_markdown

Existing Bugs: This plugin is really buggy, at least given the other stew of formatting plugins in the mix. That said, import seems to work better than in the old version. :shrug:

Maybe also notable that the markdown format is changed between these two versions.

I vote we nix this plugin. It's a mess. I can't imagine it being useful. Try it out though (on the existing package) to see.

ocdtrekkie commented 2 years ago

Are these bugs reported to the respective Etherpad repos? I guess my hope is that we can retain the features close to the original package, but ideally that Etherpad plugin updates will resolve these issues independently of our more maintainable Etherpad package.

orblivion commented 2 years ago

I reported the two New Bugs right after posting this. Maybe I could fix them, actually.

On Tue, Oct 5, 2021 at 3:10 PM Jacob Weisz @.***> wrote:

Are these bugs reported to the respective Etherpad repos? I guess my hope is that we can retain the features close to the original package, but ideally that Etherpad plugin updates will resolve these issues independently of our more maintainable Etherpad package.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sandstormports/community-project/issues/15#issuecomment-934694899, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKH6HFVWXIUPQZ4EDZCBLUFNEQVANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

orblivion commented 2 years ago

BTW I meant "caret" when I said "cursor". I corrected my comment.

The author (or one of them) of the font plugin chimed in and said he fixed the logic around the headings plugin to fix this bug. Indeed the bug does not appear there. So maybe that's an easy fix. I won't have time to look for a bit tho.

zenhack commented 2 years ago

Thanks for the update.

I'm fine with junking the markdown plugin -- indeed, I once started an etherpad doc working under the assumption that I could export it as markdown, and then was annoyed when I found the output was really bad. It would have saved me some time if it was clear that wasn't going to work well at the outset; I would have just used SandMD or something.

Re: node-capnp, I don't expect too much trouble; most of what's changed is just internals to work with newer versions of the v8 FFI.

WRT milestone bookkeeping, entirely up to you how you want to count that; you're not billing us per milestone, so whatever makes it easiest for you to keep track of things is fine with me.

orblivion commented 2 years ago

The only reason I care about milestone bookkeeping is to compare estimated and actual times. You have the option to pull the plug on this whole thing based on that, so I want to give you useful data.

On Tue, Oct 5, 2021 at 5:05 PM Ian Denhardt @.***> wrote:

Thanks for the update.

I'm fine with junking the markdown plugin -- indeed, I once started an etherpad doc working under the assumption that I could export it as markdown, and then was annoyed when I found the output was really bad. It would have saved me some time if it was clear that wasn't going to work well at the outset; I would have just used SandMD or something.

Re: node-capnp, I don't expect too much trouble; most of what's changed is just internals to work with newer versions of the v8 FFI.

WRT milestone bookkeeping, entirely up to you how you want to count that; you're not billing us per milestone, so whatever makes it easiest for you to keep track of things is fine with me.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sandstormports/community-project/issues/15#issuecomment-934834104, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKH6BVCYPRFSV3WBY3N43UFNSCJANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

zenhack commented 2 years ago

Fair enough. In any case, what you suggested is fine with me.

Quoting Daniel Krol (2021-10-05 18:03:52)

The only reason I care about milestone bookkeeping is to compare estimated and actual times. You have the option to pull the plug on this whole thing based on that, so I want to give you useful data. On Tue, Oct 5, 2021 at 5:05 PM Ian Denhardt @.***> wrote:

Thanks for the update.

I'm fine with junking the markdown plugin -- indeed, I once started an etherpad doc working under the assumption that I could export it as markdown, and then was annoyed when I found the output was really bad. It would have saved me some time if it was clear that wasn't going to work well at the outset; I would have just used SandMD or something.

Re: node-capnp, I don't expect too much trouble; most of what's changed is just internals to work with newer versions of the v8 FFI.

WRT milestone bookkeeping, entirely up to you how you want to count that; you're not billing us per milestone, so whatever makes it easiest for you to keep track of things is fine with me.

� You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/sandstormports/community-project/issues/15#issuecom ment-934834104, or unsubscribe

https://github.com/notifications/unsubscribe-auth/AAAKH6BVCYPRFSV3WBY3 N43UFNSCJANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-em ail&mt=8&pt=524675 or Android

https://play.google.com/store/apps/details?id=com.github.android&refer rer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source %3Dgithub.

-- You are receiving this because you were mentioned. Reply to this email directly, [1]view it on GitHub, or [2]unsubscribe. Triage notifications on the go with GitHub Mobile for [3]iOS or [4]Android.

Verweise

  1. https://github.com/sandstormports/community-project/issues/15#issuecomment-934914896
  2. https://github.com/notifications/unsubscribe-auth/AAGXYPV5CNMYG2JTFTK5PY3UFNY4RANCNFSM5E6W6X3Q
  3. https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
  4. https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
orblivion commented 2 years ago

Looking a little more closely at the same question with regard to the font color plugin: in our current Etherpad package, moving the caret around does not change the value of the dropdown box. In the latest version of Etherpad and the plugin, it does, though it has an issue:

If you select an area that's all red, it correctly shows up as "red". If you select an area that starts red and ends green, it correctly shows up as "Font Color", indicating that it can't say one way or the other. If you select an area that starts red, goes to green, and ends back on red, it incorrectly (imho) sets the dropdown to "red".

So, it doesn't look at every character in the selection, only the first and last character. I discovered this while looking at its implementation, while trying to fix the aforemention font size and font family bugs. The facility functions provided by etherpad to get the attributes at the various characters don't appear to be very friendly toward implementing it correctly.

Talking it over with John from Etherpad here: https://github.com/ether/ep_font_family/issues/29

Question: Supposing John wants to bring Font Size and Font Family up to par with Font Color with this regard, do we count this as a regression and stop the presses? Or is this not exactly a bug?

orblivion commented 2 years ago

I also have a worst-case proposal:

Let's say at the end of the day we don't like what the plugins are doing with the dropdowns, or maybe it will be fixed it but it'll just take more time than we want to wait/spend right now. We could fork these plugins and short-circuit them, such that it works like our old etherpad package, in that the dropdown does not reflect the selection at all.

This would be very easy to do.

ocdtrekkie commented 2 years ago

I am not super worried about non-Sandstorm-impacted plugin behavior, as hopefully Etherpad will patch it at some point, and a dependencies-updated release hopefully won't be a big deal for your new package.

If it's not data corrupting, but just behavior weirdness, I wouldn't worry too much about it.

orblivion commented 2 years ago

Okay. I'll call this milestone done and move on for now then.

zenhack commented 2 years ago

Agreed; I'm not inclined to spend time fussing with minor upstream bugs & regressions, unless they somehow disproportionately affect Sandstorm users.

orblivion commented 2 years ago

I'm working on the caching/minifying milestone now. I just want to give a heads up - I'm closing in on the estimate high end (6 hours) at this point. I feel like I'm getting there, but there seem to be lots of rickety moving parts that keep throwing me surprises.

orblivion commented 2 years ago

@zenhack I found that you have settings.json in rootfs rather than putting it in the etherpad-lite directory with a patch. (And I separately put it in a patch, and am just now discovering it in rootfs). So I should do one or the other. Any reason it ought to stay in rootfs?

zenhack commented 2 years ago

Quoting Daniel Krol (2021-10-20 12:48:50)

@.*** I found that you have settings.json in rootfs rather than putting it in the etherpad-lite directory with a patch. (And I separately put it in a patch, and am just now discovering it in rootfs). So I should do one or the other. Any reason it ought to stay in rootfs?

I don't think the difference is critical; I kindof like rootfs for config like that better conceptually, but either way will certainly work.

zenhack commented 2 years ago

Quoting Daniel Krol (2021-10-20 12:15:39)

I'm working on the caching/minifying milestone now. I just want to give a heads up - I'm closing in on the estimate high end (6 hours) at this point. I feel like I'm getting there, but there seem to be lots of rickety moving parts that keep throwing me surprises.

Thanks for keeping us posted. If something was going to go over time I'm not surprised it was this; it looked pretty hairy when I peeked at it. I think it's important though, since it has big implications for performance.

-Ian

orblivion commented 2 years ago

At this point I'm at 10 hours (well over estimate) on performance (caching / minification). I'm at a strange point where it looks like it's working from the outside, but when I console.log debug, it behaves in a way that - while repeatable/consistent - doesn't really match Etherpad's comments and/or Kenton's comments. (I started this off by porting Kenton's patch to modern Etherpad). Consulting with @zenhack we're not comfortable shipping it without knowing why it behaves how it does. I'm also not 100% done on minification and prefetching since I focused on caching, but those seem much less complicated to finish.

If we had time to spare and spend, I'd try to understand Etherpad's caching middleware better. And maybe just rewrite it for our needs, since it's rather different from vanilla Etherpad's needs. Per suggestion from @zenhack I will ping Kenton to see if he has a moment to look (if I'm lucky he'll remember something about this particular patch from years ago), and move on to other milestones for now. Hoping I can beat some more worst-case estimates and carve out more time for performance.

orblivion commented 2 years ago

Putting aside caching to make sure the rest of this update can get done in reasonable time. For now it seems that all the caching work hasn't made it any faster than simply turning off minimization. But perhaps we can make something of perf when we revisit later.

Moving on to other things: Basic sandstorm integrations and permissions are done, within estimate. I'll update the estimate numbers in a little bit.

Next, looking at the fork of ep_author_neat. My pre-estimate read was that there were new commits on top of where Kenton's version was forked from. Looking again now, there are no new tags (or npm releases) with those changes, so I'm going to assume those changes aren't release ready (or at least not required). Starting with Kenton's fork as-is, it basically works. The only problem is styling quirks.

Related question: how do we feel about line numbers? They seem to come standard now with etherpad. (The main styling issue with ep_author_neat has to do with the avatar smashing into the numbers.)

orblivion commented 2 years ago

BTW the previous Etherpad package had some code related to the Etherpad API endpoint:

https://github.com/kentonv/etherpad-lite/commit/6150fe70e2856bbd3471e3a4d1a5109e97168dbd#diff-414c13f51576e12aadeab256bc233c294997b60cd8cdbd9ac07c79208732c4bc

I didn't include it (and just blocked off the API instead) because I'm inclined to think it was never used. But, I want to doublecheck with people here.

My thinking is this: Firstly, we don't have a Sandstorm API endpoint set up in the pkgdef, so we didn't expose it that way. Secondly, it uses the sandstorm permission header, which means the only client could have been the browser. Etherpad tends to use sockets for such things, and the app still works after I block the API, and I didn't see any client code referring to the API "functions" anyway.

ocdtrekkie commented 2 years ago

IMHO, we position our Etherpad app as a Word or Docs competitor, where collaborative document editing is the key feature. I think author bubbles are probably more important than line numbers for our package, if we have to pick one or the other.

Your remarks on the API seem sensible. It'd be nice to get Kenton's thoughts if he remembers why some things were done, but if you don't see a working case for it, shelve it for now.

ocdtrekkie commented 2 years ago

As a side note, I took a brief look at Etherpad's HTTP API (https://etherpad.org/doc/v1.8.14/#index_http-api), in order to answer the question "do we want to implement this"... and I really can't even fathom. It sounds like the capabilities are mostly around managing multiple documents (of which does not apply to Sandstorm), and to a given document, has basically a get/set contents option only. So there probably isn't a rich amount of use to even strive for there.

orblivion commented 2 years ago

Update:

I took some ep_author_neat hours setting up the same git-patch system for it and ep_comments_page. I took up the rest of it fixing avatar size and positioning. I managed to preserve the line numbers. Some CSS hackery (which is not my forte, glad I could figure it out). However, there was also an avatar bug in the integrations, so I spent another hour on that. Both of those milestones are now maxed out.

Brighter news is that ep_comments_page (which I haven't spent any real time on) works pretty well out of the box. And the overhead for the git-patch is already done. Big thing is that there are some permissions issues I have to fix. And who knows if I'll find a bug. I'm not sure if I'll need all of Kenton's fixes since the plugin has changed a fair amount, particularly style-wise.

Also, the "style" milestone may be a nothing. Either not necessary or covered by the plugin work above anyway. However, I wonder about the width of the document in our current version of the Etherpad app vs the width in the version of Etherpad I'm working on now. The current version is thinner, and looks more like Google Docs or what have you. I wonder if that's what we want, or if maybe the old version was also thin and Kenton widened it on purpose? I haven't seen anything in the changelog yet that would suggest this is the case.

Aggregate hours at the bottom should now reflect ep_author_neat, profile integrations, and incomplete over-estimate caching stuff.

orblivion commented 2 years ago

I pushed the change to include ep_comments_page. I grossly underestimated what was involved since I didn't realize during estimation phase that our previous package changed the interface to view comments via click instead of mouseover. Upstream changed enough since then that it took a while to do (on top of other changes I needed to do).

So, I went way over time, but I'm just going to call it 4 hours for our budgeting purposes. EDIT: i.e. I'm only going to charge for 4.

orblivion commented 2 years ago

Updated numbers; I forgot a couple things and/or made some mistakes.

Also, I think Style is going to be 0, but I want to confirm that people think the wider margins (I think they're just from the more recent etherpad) are desired once I figure out the problems building the spk.

orblivion commented 2 years ago

I finally got around to mucking around with the spk generation problems I had. Turns out it has something to do with the fact that I kept sandstorm-pkgdef.capnp inside the .sandstorm directory. That's the right place for vagrant-spk. Turns out not so great for spk. There was an issue thread (https://github.com/sandstorm-io/sandstorm/issues/3535) about what may or may not be the spk issue I had. I chimed in earlier. I'll give some details of my fix and weird debugging symptoms there soon.

I haven't pushed the spk fix nor the latest sandstorm-files.list to the git repo but it should otherwise be up to date at the very moment. I'll keep pushing to it though. https://github.com/orblivion/etherpad-lite-sandstorm

What remains is a lot of odds and ends I think. I'd definitely want to address them before publishing. For now I'm just hoping you could LMK what you think (which I was hoping to have done while I was still developing).

I need to update dev times above but I'll keep them within stated estimates.

*EDIT: WARNING Do not use this on existing grains. See next post.

https://apps.sandstorm.io/app/95fyaurwphmn76mmcdyex2eyhj1qm6nacz4mv1r4599ktww7dzm0?experimental=true

A few notes:

On Sun, Jan 2, 2022 at 12:50 PM Daniel Krol @.***> wrote:

Updated numbers; I forgot a couple things and/or made some mistakes.

Also, I think Style is going to be 0, but I want to confirm that people think the wider margins (I think they're just from the more recent etherpad) are desired once I figure out the problems building the spk.

— Reply to this email directly, view it on GitHub https://github.com/sandstormports/community-project/issues/15#issuecomment-1003751851, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKH6FANKTNIFD7GSSUCU3UUCGAHANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

orblivion commented 2 years ago

Uh, so this should be a totally new app, but for whatever reason, existing grains are listed as grains that you can open with this one. DO NOT DO THIS. Migration from the old version is one of the things I need to fix (it's a path issue).

Looks like a Sandstorm issue. https://github.com/sandstorm-io/sandstorm/issues/3598

On Fri, Jan 7, 2022 at 4:56 PM Daniel Krol @.***> wrote:

I finally got around to mucking around with the spk generation problems I had. Turns out it has something to do with the fact that I kept sandstorm-pkgdef.capnp inside the .sandstorm directory. That's the right place for vagrant-spk. Turns out not so great for spk. There was an issue thread (

https://github.com/sandstorm-io/sandstorm/issues/3535#issuecomment-1003748003 ) about what may or may not be the spk issue I had. I chimed in earlier. I'll give some details of my fix and weird debugging symptoms there soon.

I haven't pushed the spk fix nor the latest sandstorm-files.list to the git repo but it should otherwise be up to date at the very moment. I'll keep pushing to it though. github.com/orblivion/etherpad-lite-sandstorm

What remains is a lot of odds and ends I think. I'd definitely want to address them before publishing. For now I'm just hoping you could LMK what you think (which I was hoping to have done while I was still developing).

I need to update dev times above but I'll keep them within stated estimates.

https://apps.sandstorm.io/app/95fyaurwphmn76mmcdyex2eyhj1qm6nacz4mv1r4599ktww7dzm0?experimental=true

A few notes:

  • The default theme for Etherpad is Calibris. You'll find that it looks fairly different from the current app for that reason. If we anticipate formatting compatibility problems or something, we can go back (though it'll take a little time to do some css adjustments to compensate). It looks a lot better though IMO.

  • At one point while trying this out, the spk packaged version would stall something awful on page reload. I suspect it was a missing file or something. I did an alwaysInclude and the stall seems to have gone away. lmk if you see it and apologies ahead of time if it slows your server down or something.

  • I changed the logo to what seems to be the current Etherpad logo (CC license attributed in the git README). They do still use the antenna thing some places on their site so we can switch back if you want, but it looks less like a logo IMO.

  • I would eventually like to put in some more time on startup time, but merely turning off minification puts us within the realm of reasonable for now IMO, but again chime in if it seems too slow.

On Sun, Jan 2, 2022 at 12:50 PM Daniel Krol @.***> wrote:

Updated numbers; I forgot a couple things and/or made some mistakes.

Also, I think Style is going to be 0, but I want to confirm that people think the wider margins (I think they're just from the more recent etherpad) are desired once I figure out the problems building the spk.

— Reply to this email directly, view it on GitHub < https://github.com/sandstormports/community-project/issues/15#issuecomment-1003751851 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAAKH6FANKTNIFD7GSSUCU3UUCGAHANCNFSM5E6W6X3Q

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/sandstormports/community-project/issues/15#issuecomment-1007767915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKH6HAUJUNYKEB3EAVYUTUU5OP3ANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

ocdtrekkie commented 2 years ago

Note that in my example at sandstorm-io/sandstorm#3598 you cannot open old grains with the new app. It displays them in the list for the new app, but since they're shared with me, they open with whatever package the grain owner has set for them.

orblivion commented 2 years ago

Ah yes, same experience here. That's a relief. And I suppose it makes sense; list and open are different functions.

zenhack commented 2 years ago

On my local dev instance, when I start a new grain I get a pop-up that shows:

An error occurred

Please press and hold Ctrl and press F5 to reload this page

If the problem persists, please send this error message to your webmaster:
Error: Ace2Editor.init() error event while waiting for load event
at http://ui-8e62f9f36bc376e2c86c620bd187ad6d.local.sandstorm.io:6080/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=7601783c at line 2994
ErrorId: WRyoo1coVpVsG7bm99fy
Unhandled Promise rejection
URL: http://ui-8e62f9f36bc376e2c86c620bd187ad6d.local.sandstorm.io:6080/p/main
UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0

Behind it is etherpad's loading page. Refreshing results in the same error. This shows in the grains' debug console:

[2022-01-07 17:35:34.678] [WARN] client - Error: Ace2Editor.init() error event while waiting for load event -- {
  errorId: 'WRyoo1coVpVsG7bm99fy',
  type: 'Unhandled Promise rejection',
  msg: 'Error: Ace2Editor.init() error event while waiting for load event',
  url: 'http://ui-8e62f9f36bc376e2c86c620bd187ad6d.local.sandstorm.io:6080/p/main',
  source: 'http://ui-8e62f9f36bc376e2c86c620bd187ad6d.local.sandstorm.io:6080/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=7601783c',
  linenumber: 2994,
  userAgent: 'Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0',
  stack: 'errorCb@http://ui-8e62f9f36bc376e2c86c620bd187ad6d.local.sandstorm.io:6080/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=7601783c:2994:19\n'
}

It seems to work fine on alpha though...

zenhack commented 2 years ago

Re: notes:

ocdtrekkie commented 2 years ago

I need to play a bit more this weekend, but it seemed much less quirky than our old Etherpad package at first glance. It just feels like we've moved to a modern document editor for the first time, so I'm pretty excited.

Performance seems okay, but single document grains are always best if they start super fast. Perhaps once upgrade is smoothed out a bit, we can ask Kenton to review (since he will be involved in either the key rotation or transfer), and maybe he may have some suggestions or insight on startup improvements.

orblivion commented 2 years ago

When I was debugging the cookies issue, I was trying different versions to see when it changed, and I found that there was a version not that long ago that started really fast. I wonder what the "loading" screen does and if it's something we could just skip. I made a note to look into that when it's perf time later.

On Fri, Jan 7, 2022 at 9:42 PM Jacob Weisz @.***> wrote:

I need to play a bit more this weekend, but it seemed much less quirky than our old Etherpad package at first glance. It just feels like we've moved to a modern document editor for the first time, so I'm pretty excited.

Performance seems okay, but single document grains are always best if they start super fast. Perhaps once upgrade is smoothed out a bit, we can ask Kenton to review (since he will be involved in either the key rotation or transfer), and maybe he may have some suggestions or insight on startup improvements.

— Reply to this email directly, view it on GitHub https://github.com/sandstormports/community-project/issues/15#issuecomment-1007871519, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKH6DX5XL3LMX73NH3ZVLUU6QBHANCNFSM5E6W6X3Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

orblivion commented 2 years ago

I just pushed a fix to the package to get it to build and run with vagrant-spk. This was the biggest problem that I had to work around:

https://github.com/sandstorm-io/vagrant-spk/issues/320

Check out build.sh for more.

While I have gotten spk pack to work (though I'm still figuring out details with sandstorm-files.list), vagrant-spk pack doesn't seem to work just now. Maybe for trivial reasons, I only spent a minute on it. I don't know that I'll have time to fix it myself. I came into this project with the understanding that I could do it with spk. At this point I have my other computer set up I could do either but if my time is limited I'll do whichever is easiest.

ocdtrekkie commented 2 years ago

I came into this project with the understanding that I could do it with spk.

I am absolutely good with it if it only works in spk, though if it works in both, it'd be a relatively nice perk.

I think really the only thing missing for publishing is making sure the upgrade works from the old Etherpad, and then seeing if we can get Kenton to review it, and decide how to hand off the key.

orblivion commented 2 years ago

Another issue I found (and now fixed) while fixing the vagrant-spk issues was that the app did not work on the latest version of Firefox. This was Sandstorm-specific.

My fix is a selective revert of a commit that I don't really understand. So, this would be a good one to review if anybody understands browser quirks particularly well.

Error symptom: I get a popup that tells me this error message:

Error: Ace2Editor.init() error event while waiting for load event
at http://ui-25116f71e976f7bb6facfbdf8604de2b.local.sandstorm.io:6080/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=45902aac at line 2994
ErrorId: BH7HwXFZrg2tFoykmHlb
Unhandled Promise rejection
URL: http://ui-25116f71e976f7bb6facfbdf8604de2b.local.sandstorm.io:6080/p/main
UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0

This seems related: https://github.com/ether/etherpad-lite/issues/4975 However it seems to apply to Firefox overall, whereas I'm only seeing this in Sandstorm. Or maybe I'm misreading this.

I bisected and found that it was introduced by this commit, a fix for Safari:

https://github.com/ether/etherpad-lite/commit/cb9f6d67768da5c87651c03bcc24e019574121aa

Here is my fix (undo the above only for Firefox):

https://github.com/orblivion/etherpad-lite-sandstorm/commit/2e1efa0bac338100cfa75dc207d2255a5ce92889#diff-ddc6a036badf142ae4178f202df2272d02014f8fe2853064f09972ea8ef43027R1

orblivion commented 2 years ago

I am absolutely good with it if it only works in spk, though if it works in both, it'd be a relatively nice perk.

If nothing else I could get more eyes on it now if I get stuck on something.

I think really the only thing missing for publishing is making sure the upgrade works from the old Etherpad, and then seeing if we can get Kenton to review it, and decide how to hand off the key.

I've actually already made a sqlite3 file with the previous sandstorm app and put it into a testing directory for future regression testing. I even made a dirty.db file from the very first version (that you helped me find) for the same reason. Turns out that one migrates mostly fine as well. And, I've already written code for backing up the sqlite3 file when we switch to this version of the app (I change the db file name so I know when we switch versions, as you suggested).

However, right now the paths within the app are totally different from Kenton's. Part of it is understandable because the app is not a fork of etherpad anymore, but part of it may be quirks in the files list system. I need to get back to figuring that out now.

Beyond that, though, I've collected a long list of small issues that I've passed on along the way in my rush to hit the bullet points. I'm probably gonna post them soon so we can evaluate. Given track record I imagine a lot of these you guys will dismiss as nbd but I want it on the record, for future improvements if nothing else.

zenhack commented 2 years ago

Hm, I am still seeing that error in firefox when I pull your latest branch and do vagrant-spk dev. Also I am not seeing that error on alpha with the previous spk, even in firefox. It does work in chromium.

Re: vagrant-spk pack: I think it would be worth making a token effort to get it working, but I am fine punting if it looks like it's going to be another rabbit hole, as long as we're able to build it somehow.

Getting close, I'm excited.

orblivion commented 2 years ago

The aforementioned long list of small issues that are worth evaluation:

https://github.com/orblivion/etherpad-lite-sandstorm/blob/main/ISSUES.md