umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

Update RTE to use data-UDI attribute for images #5123

Closed warrenbuckley closed 5 years ago

warrenbuckley commented 5 years ago

On load RTE should replace src tag based on the value of the data-udi attribute & should be done in C# as opposed to dealing with JS replacement

It also should take into account the image width/height attributes and append the ImageProcessor querystrings to the image source.

On save it should remove the src attribute of image elements so that we always continue to use the data-UDI attribute as the real source.

rasmusjp commented 5 years ago

Looks like it's already implemented with https://github.com/umbraco/Umbraco-CMS/pull/4001, only difference is that the src isn't removed, but reading the comments I'm not sure if that's desired or not.

dawoe commented 5 years ago

I had a PR merged and reverted for V7 for removing the data-udi attributes. Maybe you can read the discussion there as well : https://github.com/umbraco/Umbraco-CMS/pull/2531

Dave

Shazwazza commented 5 years ago

@rasmusjp Regarding this one, I think there's still some work we should do for this. As @dawoe mentions, there was a PR to remove the data-udi attributes in v7 but it was reverted in favor of being implemented for v8 which i think we should still do while v8 is still young.

Having the data-udi attributes rendered on the front-end is sort of a security risk like @dawoe mentions in his links so ideally we don't render these at all.

I've also updated the name of this issue so it's more clear what it's for (for images right?)

As I'm working through https://github.com/umbraco/Umbraco-CMS/issues/4940 which makes sure that we use the correct UDI syntax for media references in links (a tags), the change for rendered images should follow the same principles IMO? If we aren't saving the /media/123123/etc path in the links, then we should be consistent and not persist these for the images too ?

Shazwazza commented 5 years ago

I have added the 'breaking' category to this since we are modifying how the html output was.

Here's an example of how the values are peristed to DB and how it is output with an rte value having both links and images in them:

RTE with image and links as saved in the DB:

<p>Hello Apple pie bonbon apple pie cookie jelly. <a rel="noopener" href="/{localLink:umb://document/540a5f0a6a154dd2affbff143b86e1ef}" target="_blank" title="Testing">Halvah</a> tart pastry powder pudding cake cotton candy lemon drops jelly-o. Candy canes chupa chups caramels. <a rel="noopener" href="/{localLink:umb://document/ca4249ed2b234337b52263cabe5587d1}" target="_blank" title="Home (1)">Chocolate marshmallow</a> lemon drops. </p>
<p>Brownie tootsie roll marzipan cupcake dragée marshmallow candy canes powder cupcake. Jelly-o pudding bear claw macaroon biscuit. Marzipan bear claw brownie cupcake powder macaroon gummies macaroon tart. Gingerbread sweet candy canes sweet fruitcake.</p>
<p><img style="width: 500px; height:375px;" src="?width=500&amp;height=375" alt="" data-udi="umb://media/7b2953520e2c40dfaf2dc320c923c322" /></p>
<p>Fruitcake bear claw oat cake cake oat cake bear claw wafer chupa chups. Toffee bear claw jelly cake fruitcake cupcake gummi bears lemon drops. Candy canes fruitcake macaroon apple pie cookie pie sweet roll. Marshmallow topping cake.</p>

how it is rendered

<p>Hello Apple pie bonbon apple pie cookie jelly. <a rel="noopener" href="/en/testing/" target="_blank" title="Testing">Halvah</a> tart pastry powder pudding cake cotton candy lemon drops jelly-o. Candy canes chupa chups caramels. <a rel="noopener" href="/home-1/" target="_blank" title="Home (1)">Chocolate marshmallow</a> lemon drops. </p>
<p>Brownie tootsie roll marzipan cupcake dragée marshmallow candy canes powder cupcake. Jelly-o pudding bear claw macaroon biscuit. Marzipan bear claw brownie cupcake powder macaroon gummies macaroon tart. Gingerbread sweet candy canes sweet fruitcake.</p>
<p><img style="width: 500px; height:375px;" src="/media/7b2953520e2c40dfaf2dc320c923c322/00000006000000000000000000000000/img_4717.jpg?width=500&amp;height=375" alt=""></p>
<p>Fruitcake bear claw oat cake cake oat cake bear claw wafer chupa chups. Toffee bear claw jelly cake fruitcake cupcake gummi bears lemon drops. Candy canes fruitcake macaroon apple pie cookie pie sweet roll. Marshmallow topping cake.</p>