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

Problem processing the following html #127

Open
nexon33 opened this issue Aug 28, 2023 · 7 comments
Open

Problem processing the following html #127

nexon33 opened this issue Aug 28, 2023 · 7 comments

Comments

@nexon33
Copy link

nexon33 commented Aug 28, 2023

When I calculate the diff using the following code snippets I run into some trouble, for some reason multiline strings are not handled accordingly. I first calculate the diff and then apply the diff on an empty div.

when I use

<div data-interchange="[/some/link, (default)],
                                [/some/other/link, (medium)]">
</div>

I get the error

DOMException: Failed to execute 'setAttribute' on 'Element': '(default)],' is not a valid attribute name. and when I use

<div data-interchange="
        [/some/link, (default)],
                                [/some/other/link, (medium)]">
</div>

I get

DOMException: Failed to execute 'setAttribute' on 'Element': '[' is not a valid attribute name.

but when I use

<div data-interchange="[/some/link, (default)], [/some/other/link, (medium)]">
</div>

then I get no error.

@johanneswilm
Copy link
Member

johanneswilm commented Aug 28, 2023 via email

@nexon33
Copy link
Author

nexon33 commented Aug 28, 2023

Here is an example using JSfiddle, as you can see the diff contains invalid actions, more specifically you can see how it detects [ as an attribute which later on causes the previously mentioned error:

{"nodeName":"div","attributes":{"[":"","(default)],":"","data-uuid":"interchange-llv1q00ub"},"childNodes":[{"nodeName":"#text","data":"\n "}]},{"nodeName":"#text","data":"\n "}]}}

https://jsfiddle.net/pbf35dov/9/

@nexon33
Copy link
Author

nexon33 commented Aug 30, 2023

I found some other code that doesn't get diffed correctly;

 <style id="style">
         .test {
        background: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48"><path d="M17.17 32.92l9.17-9.17-9.17-9.17L20 11.75l12 12-12 12z"/><path d="M0-.25h48v48H0z" fill="none"/><\/svg>') 50% 51% no-repeat;
        -webkit-background-size: 24px 24px;
        background-size: 24px 24px;
        height: 56px;
        position: relative;
        text-align: right
    }
    </style>
produces a malformed diff because of the < > characters: 

[
    {
        "action": "modifyAttribute",
        "route": [],
        "name": "id",
        "oldValue": "style",
        "newValue": "copiedstyle"
    },
    {
        "action": "modifyTextElement",
        "route": [
            0
        ],
        "oldValue": "\n         .test {\n        background: url('data:image/svg+xml;utf8,",
        "newValue": "\n\n    "
    },
    {
        "action": "removeElement",
        "route": [
            1
        ],
        "element": {
            "nodeName": "svg",
            "attributes": {
                "xmlns": "http://www.w3.org/2000/svg",
                "viewBox": "0 0 48 48"
            },
            "childNodes": [
                {
                    "nodeName": "path",
                    "attributes": {
                        "d": "M17.17 32.92l9.17-9.17-9.17-9.17L20 11.75l12 12-12 12z"
                    }
                },
                {
                    "nodeName": "path",
                    "attributes": {
                        "d": "M0-.25h48v48H0z",
                        "fill": "none"
                    }
                }
            ]
        }
    }
]

I'll make a jsFiddle tomorrow as its pretty late

@johanneswilm
Copy link
Member

Hey @nexon33 ,
do you actually need to diff those elements? In that case, why don't you just diff document.getElementById('ifr').contentDocument.body instead? Reading HTML is difficult and there will always be issues with very complex HTML even if we get this fixed. Reading from the DOM element does not have those same issues.

@nexon33
Copy link
Author

nexon33 commented Sep 6, 2023

Hey @nexon33 ,
do you actually need to diff those elements? In that case, why don't you just diff document.getElementById('ifr').contentDocument.body instead? Reading HTML is difficult and there will always be issues with very complex HTML even if we get this fixed. Reading from the DOM element does not have those same issues.

If I diff on documentbody.body then (assuming the style tag is inside the body) i still get the same problem. I really like the library and think its awesome. I'd help with the library if I could and in fact might look into actually doing that.

@johanneswilm
Copy link
Member

@nexon33 You mean document.body?

In general, the HTML parsing issues have all been fixable until now, so this one is probably as well. It's a matter of spending the time needed on that. I am currently very busy with conferences, etc., so if you can work on it, that would be very much appreciated!

@dav-q
Copy link

dav-q commented Oct 28, 2023

Recently I encountered an error similar to this:

Environment

diff-dom : 5.0.7

Behaviour

I have two strings and I want to compare them using the following code:

dd = new diffDOM.DiffDOM()
const elementA = document.createElement('div');
const elementB = document.createElement('div');
// The following strings are returned from the API, suppose you can't control this
elementA.innerHTML = `<span>Random text</span>`;
elementB.innerHTML =  `<ul><li><span data-random-attr='{"event":"mom\'s birthday"}'>Random text</span></li></ul>`;

After creating a diff ( diff = dd.diff(elementA, elementB) ), the node looks like this:

image

Finally, after applying the diff ( dd.apply(elementA, diff) ), the error happens:

Uncaught DOMException: Failed to execute 'setAttribute' on 'Element': 'birthday"}'' is not a valid attribute name.

So, The error happens because the node created in the elementB is wrong caused by special characters.

A partial solution could be to sanitize the strings using dompurify and the error will disappear

Maybe this can help someone

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

No branches or pull requests

3 participants