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

Add html component #237

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Add html component #237

merged 1 commit into from
Jun 10, 2024

Conversation

wwwillchen
Copy link
Collaborator

@wwwillchen wwwillchen commented May 11, 2024

This allows you to render sanitized HTML, however it doesn't allow potentially dangerous HTML like JavaScript.

In a separate PR, I'll update the embed component so you can essentially do an <iframe srcdoc="$htmlcontent">, which will allow you to run arbitrary JS in a sandboxed way.

Partially addresses #156.

@richard-to
Copy link
Collaborator

  1. There's no styling inside the iframe. Should we inject the stylesheet in here?

What do you mean by "inject the stylesheet"?

When using the iframe with source doc, is the user able to reference assets (Javascript, CSS) from other domains or would they need to reference from same domain? Or is that not allowed for security reasons?

I guess if linking to external resources is not allowed, then one could write the js/css in the html head. Probably not that user friendly, but since this is an advanced use case, probably ok. Although would be hard to manage.

  1. The iframe height is arbitrary (browser default). Should we try to automatically resize the iframe based on the content or should Mesop app developers set a height?

I think the app developer should able to set the height. So if there is auto resize as a default and the user could resize as an override, that may be more convenient.

Copy link
Collaborator

@richard-to richard-to left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


## Examples

<iframe class="component-demo" src="https://mesop-y677hytkra-uc.a.run.app/link"></iframe>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Looks like these are pointing to "link" when it should be html?

@wwwillchen
Copy link
Collaborator Author

will re-open once I've been able to address the comments.

@wwwillchen wwwillchen closed this May 19, 2024
@wwwillchen wwwillchen mentioned this pull request Jun 5, 2024
@wwwillchen wwwillchen reopened this Jun 10, 2024
@wwwillchen wwwillchen changed the title Add HTML component Add html component and embed(html=...) option Jun 10, 2024
@wwwillchen wwwillchen changed the title Add html component and embed(html=...) option Add html component Jun 10, 2024
@wwwillchen
Copy link
Collaborator Author

@richard-to could you take another look? I updated this PR quite a bit and switched to sanitizing the HTML instead of iframing the HTML (I'll try this approach in a separate #405). This is better for some use cases because it gets rendered in the same frame, but it doesn't support JS so it's not that flexible.

Copy link
Collaborator

@richard-to richard-to left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Just to make sure I understand. The HTML component here renders HTML directly without an iframe. But it is sanitized so no javascript and other malicious stuff can be added?

mesop/components/html/html.ts Outdated Show resolved Hide resolved
mesop/components/html/html.ts Outdated Show resolved Hide resolved
mesop/web/src/safe_iframe/safe_iframe.ts Outdated Show resolved Hide resolved
mesop/components/html/html.ng.html Show resolved Hide resolved
@wwwillchen
Copy link
Collaborator Author

LGTM - Just to make sure I understand. The HTML component here renders HTML directly without an iframe. But it is sanitized so no javascript and other malicious stuff can be added?

Yup, that's right. Right now we've been telling people to use me.markdown as a way to render sanitized HTML, which is 1) kind of confusing (since it's not obvious to most developers) and 2) kind of annoying since markdown adds a <p> which adds margin, and having a raw div makes it more straightforward to style.

I think we still need JS for a bunch of use cases, so we'll need to handle that separately.

@wwwillchen wwwillchen merged commit 75ccefa into google:main Jun 10, 2024
3 checks passed
@wwwillchen wwwillchen deleted the html branch June 10, 2024 21:21
wwwillchen added a commit to wwwillchen/mesop that referenced this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants