Modify clipboard override options and add application/x-latex to default copy/cut#342
Modify clipboard override options and add application/x-latex to default copy/cut#342
application/x-latex to default copy/cut#342Conversation
…they did anything or not
application/x-latex to default copy/cut
|
The test that is failing is for "cut is async" so it seems we are explicitly expecting that. I'm a little hesitant to remove the delay and make it synchronous just out of concern for regressions and what seems like little upside. I think we should leave it. |
|
I wonder if there are any tests we should write here. I imagine we are changing some kind of behavior that should get tested somehow. |
| if (earlyReturn) return; | ||
| } | ||
| updateClipboardData(clipboardEvent); | ||
| everyTick.listenOnce(function () { |
There was a problem hiding this comment.
I think maybe jared addressed this, but it seems odd to me to do both the controller.cut() and the updateClipboardData() here. I don't really feel like I have a good handle on what's going to happen in practice. Is the updateClipboardData() going to work or not? If it does, then what do we need the controller.cut() for? Is the controller.cut() going to overwrite whatever the updateClipboardData() does?
My main concern here is just making sure all mqs that don't use the override methods continue to work correctly. I remember recently digging into copy/paste in the knox acceptance tests and realizing that the timing of things is really subtle.
There was a problem hiding this comment.
In the previous code when you did a cut, you were actually using the browser default behavior and cutting text out of a textarea while MQ would remove the latex from display. I think this is reason for the subtle timing weirdness. The browser has to finish the cut from the textarea before MQ can clean up everything.
In this code, we're manually updating the clipboard data, calling preventDefault() so the textarea thing is irrelevant, but controller.cut() is still necessary to have MQ remove the latex. I think this simplifies the timing, but I'm leaving the extra tick around the cut just to avoid unforeseen regressions.
What do you think?
mikehaverstock
left a comment
There was a problem hiding this comment.
I think we'll want to manually test this within knox anywhere we aren't using the override methods. There's a chance that copy/paste could be subtly broken there and it not be caught by tests. I don't remember offhand which kinds of interactions are most likely to break, but I vaguely remember something where we intercept a paste to do something different with the paste. I imagine there's room for that to be broken. And maybe it'd be better to use these override methods now.
I guess just to sanity check my thoughts here... Are we planning on some mathquills using the overrides and some not? Or are all going to use the overrides once this is merged?
The intent is to allow the overrides to be used or not. It should work without them just fine and I don't think we will use them everywhere (or need to). I definitely agree that testing it thoroughly is a good idea though. |
This change updates
overrideCopy,overrideCut, andoverridePasteto have boolean return values so they can more easily decide if the default behavior should or shouldn't run.This also changes the default copy/cut behavior to use the clipboard API instead of relying on the textarea + default browser copy behavior. That allows us to add
application/x-latexas a mime type on the clipboard so other code that is pasting it can know it's latex.