kfahy / slack-disable-wysiwyg-bookmarklet

Disables the WYSIWYG editor in Slack.
MIT License
824 stars 23 forks source link

Interested in a persistent script to patch Desktop slack? #15

Closed dbalatero closed 4 years ago

dbalatero commented 4 years ago

Hi @kfahy !

I wrote a Bash script that will patch your Mac version of Slack here:

https://github.com/dbalatero/dotfiles/blob/master/bin/slack-disable-wysiwyg

It is idempotent and will not patch it a second time, but you can keep running it after each Slack update.

Any interest in merging this in as an official desktop solution?

kfahy commented 4 years ago

Amazing. This looks great! Yes, I'd love a PR for this, if you'd be willing.

dbuskariol commented 4 years ago

Hey @dbalatero 👋

Thanks for writing this script ✨ I was in the process of figuring this out by modifying ssb-interop.bundle.js but hadn't yet turned it into a script. For some reason though this isn't working for me. It runs fine, and won't re-patch saying the patch already exists but it doesn't seem to actually disable it in Slack.

dbalatero commented 4 years ago

Hi! If you go to the resources directory and run the asar unpack command in my script manually what do you see in the file?

On Thu, Nov 21, 2019 at 3:39 PM Daniel Buskariol notifications@github.com wrote:

Hey @dbalatero https://github.com/dbalatero 👋

Thanks for writing this script ✨ I was in the process of figuring this out by modifying ssb-interop.bundle.js but hadn't yet turned it into a script. For some reason though this isn't working for me. It runs fine, and won't re-patch saying the patch already exists but it doesn't seem to actually disable it in Slack.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/kfahy/slack-disable-wysiwyg-bookmarklet/issues/15?email_source=notifications&email_token=AAAOQJJ7I2VJRTXFJLLSGF3QU4L2DA5CNFSM4JQHIPH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4A4YA#issuecomment-557321824, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOQJPT6R2AR645SUZ7TBDQU4L2DANCNFSM4JQHIPHQ .

dbuskariol commented 4 years ago

@dbalatero I see the ssb-interop.bundle.js has been modified with your solution. This is end tail of ssb-interop.bundle.js:

length;++index<length;){var current=iteratee(array[index]);void 0!==current&&(result=void 0===result?current:result+current)}return result};__webpack_exports__.a=function sumBy(array,iteratee){return array&&array.length?_baseSum(array,Object(_baseIteratee.a)(iteratee,2)):0}}]);
//# sourceMappingURL=ssb-interop.bundle.js.map
// WYSIWYG patch

const disableWysiwygEditor = (_ => {
  const redux = slackDebug[slackDebug.activeTeamId].redux;

  const {
    wysiwyg_composer,
    wysiwyg_composer_ios,
    wysiwyg_composer_webapp,
    ...payload
  } = redux.getState().experiments;

  redux.dispatch(
    {
      type: '[19] Bulk add experiment assignments to redux',
      payload
    }
  );
});

const disableInterval = setInterval(() => {
  const isLoaded = window.slackDebug &&
    window.slackDebug[window.slackDebug.activeTeamId] &&
    window.slackDebug[window.slackDebug.activeTeamId].redux;

  if (isLoaded) {
    clearInterval(disableInterval);
    setTimeout(disableWysiwygEditor, 2000);
  }
}, 50);
dbalatero commented 4 years ago

@dbuskariol wheeeeeeee ok!

Can you crank that setTimeout from 2000 ms to say 10000? And see if it happens?

Also let's get more debug info. If you run Slack like this:

export SLACK_DEVELOPER_MENU=true
/Applications/Slack.app/Contents/MacOS/Slack &

You should be able to right click in Slack and "Inspect Element" to bring up the console. Can you look for any errors?

Also before you do that, can you add to the interop file some console logs?

Maybe:

// WYSIWYG patch

const disableWysiwygEditor = (_ => {
  const redux = slackDebug[slackDebug.activeTeamId].redux;

  const {
    wysiwyg_composer,
    wysiwyg_composer_ios,
    wysiwyg_composer_webapp,
    ...payload
  } = redux.getState().experiments;

  console.log("========> dispatching");

  redux.dispatch(
    {
      type: '[19] Bulk add experiment assignments to redux',
      payload
    }
  );
});

const disableInterval = setInterval(() => {
  const isLoaded = window.slackDebug &&
    window.slackDebug[window.slackDebug.activeTeamId] &&
    window.slackDebug[window.slackDebug.activeTeamId].redux;

  if (isLoaded) {
    console.log("======> setting timeout!");
    clearInterval(disableInterval);
    setTimeout(disableWysiwygEditor, 10000);
  }
}, 50);

any time you modify the file you'll need to run (from the Resources dir):

sudo "PATH=$PATH npx asar pack app.asar.unpacked app.asar

and restart Slack.

pocc commented 4 years ago

Agree with @dbalatero. In my testing, a minimum of 3000 is required, but I use 5000 for the extension. 10000 means the user is waiting for 10s, so a number that high might not be desirable.

dbalatero commented 4 years ago

Ah yeah to clarify, I just wanted to bump to 10000 to test, but we should definitely find where the sweet spot is.

Also I don't love repatching the JS each time we need to tweak the timing.

I'd rather have the interop.js file contain:

require(process.env.HOME + "/.slack-inject.js")

and then edit that file directly. I'm not sure that require() can be called in the context of that file though… it probably isn't running in Node (unless asar pack resolves require() calls, but that would still require re-patching anyways). hmm.

dbuskariol commented 4 years ago

Hey @dbalatero and @pocc, thanks–that was my next line of thinking. I've upped it to 4000 and it's working great ✨

dbuskariol commented 4 years ago

I guess the next step would be figuring out how to get it to persist across workspaces 🤔

dbalatero commented 4 years ago

I wonder if instead of firing it for the activeTeamId we just loop through the array instead and fire for all redux instances?

On Thu, Nov 21, 2019 at 6:53 PM Daniel Buskariol notifications@github.com wrote:

I guess the next step would be figuring out how to get it to persist across workspaces 🤔

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kfahy/slack-disable-wysiwyg-bookmarklet/issues/15?email_source=notifications&email_token=AAAOQJOS2F5RQ3FRJY6GTB3QU5CTRA5CNFSM4JQHIPH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4LPDI#issuecomment-557365133, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOQJJVP4GX7YZZ7B6OWLLQU5CTRANCNFSM4JQHIPHQ .

kfahy commented 4 years ago

Yeah that seems like the better play. Check out this code snippet from @tdkn.

bripkens commented 4 years ago

When you move your Slack.app to $HOME/Applications beforehand, then you can avoid execution with sudo and also remove sudo from the script. This makes this a lot less dangerous.

dbalatero commented 4 years ago

Sounds good to me, can it be temporarily moved and then moved back again?

On Thu, Nov 21, 2019 at 10:59 PM Ben Blackmore notifications@github.com wrote:

When you move your Slack.app to $HOME/Applications beforehand, then you can avoid execution with sudo and also remove sudo from the script. This makes this a lot less dangerous.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kfahy/slack-disable-wysiwyg-bookmarklet/issues/15?email_source=notifications&email_token=AAAOQJK2GFXNWE6DCUSCMELQU57MJA5CNFSM4JQHIPH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4X4RY#issuecomment-557416007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOQJMVFHGISLVIURVAVSTQU57MJANCNFSM4JQHIPHQ .

dbalatero commented 4 years ago

@dbuskariol @kfahy

I have a new script that fires the disable code the first time you switch to any team. There is a 5000ms timeout, so it'll take a second to pop in.

https://github.com/dbalatero/dotfiles/blob/master/bin/slack-disable-wysiwyg

kfahy commented 4 years ago

Cool stuff! A couple questions:

  1. Instead of executing when switching teams, would it also work to call getAllWorkspaces() and dispatch to each redux instance at once? I updated index.js to take this approach, adapted from #2, so if that doesn't work for some cases, I can revert.
  2. Are we waiting for 5000ms to let the experiments initially load? If there's any variance to this time, maybe we could also key off of the presence of the WYSIWYG formatting button, e.g. document.querySelector('[data-qa="texty_composer_button"]'). It'll be present after slack's experiments load, at which time it's safe to overwrite the experiments with our dispatch call.
dbalatero commented 4 years ago

Instead of executing when switching teams, would it also work to call getAllWorkspaces() and dispatch to each redux instance at once? I updated index.js to take this approach, adapted from #2, so if that doesn't work for some cases, I can revert.

What I noticed (I cannot confirm though) is that it seemed like each workspace's Redux didn't bootstrap itself until you actually switched. It was more stable once I started to hook in this way.

Are we waiting for 5000ms to let the experiments initially load? If there's any variance to this time, maybe we could also key off of the presence of the WYSIWYG formatting button, e.g. document.querySelector('[data-qa="texty_composer_button"]'). It'll be present after slack's experiments load, at which time it's safe to overwrite the experiments with our dispatch call.

Yes, oh nice let's definitely do that! I will patch now.

dbalatero commented 4 years ago

@kfahy patch is updated, and it is almost instant now!

https://github.com/dbalatero/dotfiles/blob/master/bin/slack-disable-wysiwyg

Can you test and let me know if it works?

I will update the patch script to drop the sudo requirement now.

dbalatero commented 4 years ago

I will update the patch script to drop the sudo requirement now.

Nevermind, I can't actually do that effectively. I'm leaving it as is, anyone who is paranoid about sudo can audit the script.

I added a little warning when it prompts for password, and encourages people to audit the script.

kfahy commented 4 years ago

Sweet! Tested it out with two workspaces and it looks good to me. Thanks a lot!

Would you prefer that we link to your repo in the readme, or add the script to this one? Will leave up to you - feel free to submit a PR either way.

dbalatero commented 4 years ago

@kfahy i'll open a PR, np