Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnTeX Action #187

Open
pihart opened this issue Jun 17, 2021 · 7 comments
Open

UnTeX Action #187

pihart opened this issue Jun 17, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@pihart
Copy link
Collaborator

pihart commented Jun 17, 2021

When an image that was inserted from qboard's tex system is selected, users should be able to run an action that recovers the original tex source. This should also be in the image's context menu (#203; see also #28). This should work even if the file is saved and reopened.

NOTE: while it's good to have it in the context menu, this issue only concerns the action behind it. See #203 for the "full" version of this issue, where we also add to the context menu. That issue is likely dependent on #28;

Solution:

  • When adding an image from the tex action, tag it with a data field containing the original source.
    • We can do this by adding a data tag to the generated svg before importing; it should be preserved when serialized. Note that this would require us to parse the svg+xml as xml, which for most svgs is a very small but nonzero amount of work. The next option doesn't require this.
    • Alternatively, we can tell fabric itself what the source was by giving it to a field in the fabric.Image element. Figure out how to make sure it gets serialized and reinserted.
      • Make sure to make a custom type TeXImage or something so you don't have a bunch of floating anys and invalid lookups. The operation that checks whether the image came from tex should be a type guard for this type.
  • The action should check that there is a selection and that it is valid by whatever spec we define. Then extract the tex and return it according to whatever spec.
    • It can take a flag so that if we know that the selection is already an image or image from tex we don't need to check again.
  • For any image selection from tex (or whatever spec), there should be a context menu item that invokes the action.

Spec decision:

  • What to do when the selection contains more than one valid image file
    • You could just have it stop checking when it realizes there is more than one item in the selection. This seems like an unnecessary restriction
    • You could have it output the tex values of all valid images from tex in some sensible way
  • How to return the value?
    • console
    • alert + console
    • clipboard copy + console
      • What would this copy to the clipboard if there is more than one admissible image file?
      • Do we give a toast indicating that the copy has been done?

This is again not a security concern because we never render arbitrary tex (except obviously when the immediate user requests that we do so with the tex action).

However, we may want to be mindful that someone may make a fake JSON file where they set the tex data to a wrong value. I think this is fine; the user should always validate their own TeX before running it, but we might want to give a warning that the returned tex is not necessarily correct.

Keep in mind that if you actually do something about this, you also want to cleanse every other SVG added to the board. Also, it is not a valid solution to check the validity of the tex field by running it and comparing the outputs, because the whole security concern is that you don't want to run it (we don't trust mathjax enough to say that it doesn't have an XSS or blocking operation vulnerability). I don't think this is something to worry about, though (just like it's not Google Docs's job to ensure that what someone writes in a comment is truthful).

@pihart pihart added the enhancement New feature or request label Jun 17, 2021
@cjquines
Copy link
Owner

see #188 for my feelings on this

@cjquines
Copy link
Owner

cjquines commented Jun 18, 2021

in particular i do not see any use case at all for this. notably, this is not a feature requested in anything because there is no use case and i will insist there is no use case

@cjquines cjquines added the won't fix This will not be worked on label Jun 18, 2021
@pihart
Copy link
Collaborator Author

pihart commented Jun 18, 2021

see #188 for my feelings on this

I disagree; this feature is just reporting information that we already have.

in particular i do not see any use case at all for this. notably, this is not a feature requested in anything because there is no use case and i will insist there is no use case

I have wanted to edit formulas/text I've added to qboard many times. This let's us do precisely that.

@cjquines
Copy link
Owner

I have wanted to edit formulas/text I've added to qboard many times

okay, sure, that's a use case. and yet, i don't think there's any good ui for this. nothing in qboard currently allows you to edit it once it's placed. change the color of something you drew? idk. maybe. but still

@pihart
Copy link
Collaborator Author

pihart commented Jun 18, 2021

i don't think there's any good ui for this.

Just an action is sufficient. The bulk of the work is then #193, which is already solved by #194.

nothing in qboard currently allows you to edit it once it's placed. change the color of something you drew? idk. maybe. but still

This is precisely #28. If we want to add anything beyond an action (we might not, and I'm personally fine that way) then we'll just piggy-back on that idea, with #28 ideally but not necessarily resolved first.

@cjquines cjquines removed the won't fix This will not be worked on label Jun 20, 2021
@cjquines
Copy link
Owner

i think this is okay now, actually, but i think this is blocked by #28 first

@pihart pihart changed the title UnTeX option UnTeX Action Jun 20, 2021
@pihart
Copy link
Collaborator Author

pihart commented Jun 20, 2021

i think this is blocked by #28 first

Changed this issue to be just the action; context menu part has been moved to #203. This present issue is now resolvable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants