detecting changes in form fields about diffdom

[johanneswilm]

Yes, doesn't this change the DOM? Check the sources for the live demo, it does that.
diffDOM just creates diffs. You have to write your own code to trigger such diffing when you need it.

[Elexy]

No it doesn't change the dom, as far as I can see the attributes are compared and changing the VALUE of an input through user interaction does not change the VALUE attribute.
I can do this before I feed the html element to the diff, but I would like to be able to change html to obj once, then update it with the correct input values and the do the diff.

So really being able to feed the diff objects instead AND/OR dom Elements would be ideal.

This is why I was initially converting html to object separately and used https://github.com/benjamine/JsonDiffPatch manually.

I'll study a bit how I can use your lib....

[johanneswilm]

Can you make an exact list of all HTML elements that are affected by this? (ignoring innerHTML and using value instead)

[Elexy]

What I've come up with so far is these:
input: value
radio / checkbox: checked
select->option: selected
textarea: text

I was looking at how this can be done with current setup. Currently the dom elements are used to diff against. I'm not sure yet, but it seems adding these would mean adding checks for these element properties and adding them to the attributes so the can be diffed / transformed into objects.

[marcelklehr]

The notable distinction here is that attributes != properties.
Attributes reflect the HTML tag attributes, while DOM node properties
are changed dynamically.

I don't know which of the two diffDOM deals with.

[johanneswilm]

hold on, I'm trying something out. should have something to test soon.

[johanneswilm]

Could you guys try this latest change? This should work for changes to values. I've tried it with textareas. It will possibly also work for value changes to input fields. radio/checkbox/selected will have to be done similarly but separate.

[Elexy]

will have a go now!

[Elexy]

preliminary findings:
send Chrome into meltdown, cpu spins up and becomes unresponsive.
on a page with several inputs all goes well until i change an input.

[johanneswilm]

Ok, could you try with textareas only?

On Tue, Feb 25, 2014 at 4:57 PM, Alex Knol [email protected] wrote:

preliminary findings:
send Chrome into meltdown, cpu spins up and becomes unresponsive.
on a page with several inputs all goes well until i change an input.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-36050796
.

Johannes Wilm
http://www.johanneswilm.org
tel US: [phone]
tel NO: [phone]
tel DE: [phone]

[Elexy]

done; no hangups, but keeps finding diffs every pass:
changing text from 'some text' to 'hjgjhgjhg jkh jg jhgk '
[{"action":"modify value","oldValue":"some text","newValue":"hjgjhgjhg jkh jg jhgk ","route":[2,3]}]
now changing from 'hjgjhgjhg jkh jg jhgk ' to 'some text more':
``[{"action":"modify value","oldValue":"some text","newValue":"some text more","route":[2,3]}]`

[johanneswilm]

Could you be more specific? Why shouldn't it find diffs in the second round?

On Tue, Feb 25, 2014 at 5:03 PM, Alex Knol [email protected] wrote:

done; no hangups, but keeps finding diffs every pass:
[{"action":"modify value","oldValue":"some text","newValue":"hjgjhgjhg jkh
jg jhgk ","route":[2,3]}]

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-36051378
.

Johannes Wilm
http://www.johanneswilm.org
tel US: [phone]
tel NO: [phone]
tel DE: [phone]

[Elexy]

Basically always diffing with the original value, not the changed (now current) value.

Currently we take the inputs, read the currently value/text/checked/etc property and write it in the respective attribute. This way it becomes the new 'current' value for the next diff

[johanneswilm]

Have you applied the diff? If you just create the diff without applying it,
it should create the same diff again if you run it again.

On Tue, Feb 25, 2014 at 5:06 PM, Alex Knol [email protected] wrote:

Basically always diffing with the original value, not the changed (now
current) value.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-36051732
.

Johannes Wilm
http://www.johanneswilm.org
tel US: [phone]
tel NO: [phone]
tel DE: [phone]

[johanneswilm]

also note: if you use cloneNode in your test -- it is not working correctly
on textareas. It is resetting the value of the textarea to the original
value.

On Tue, Feb 25, 2014 at 5:07 PM, Johannes Wilm [email protected]:

Have you applied the diff? If you just create the diff without applying
it, it should create the same diff again if you run it again.

On Tue, Feb 25, 2014 at 5:06 PM, Alex Knol [email protected]:

Basically always diffing with the original value, not the changed (now
current) value.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-36051732
.

Johannes Wilm
http://www.johanneswilm.org
tel US: [phone]
tel NO: [phone]
tel DE: [phone]

Johannes Wilm
http://www.johanneswilm.org
tel US: [phone]
tel NO: [phone]
tel DE: [phone]

[Elexy]

I am using cloneNode. I see your point about applying the patch, but It's not viable for me, as I can't touch the original DOM. I'll check some more...

[johanneswilm]

if you don't apply the diff (= change one of the elements), then why should it behave differently when
comparing the same elements a second time?

[johanneswilm]

Maybe you should explain what exactly you want to do. Maybe I can help you, maybe I can't.

[Elexy]

I need to send the entire HTML over the line and build it up on the other side exactly as it's displayed on the first screen.

[johanneswilm]

could you also file the file with the text input fields that gave you errors? some basic testing with text input fields here didn't give me errors.

[johanneswilm]

ok, great, but then you need exactly this functionality as it is now with textareas, right?
What are you comparing? You cannot use pure cloneNode to ave an exact copy, because there is a bug in it when working with textareas, as I mentioned above. You have to create a function such as https://github.com/johanneswilm/diffDOM/blob/gh-pages/diffDOM.js#L73-L92 .

[Elexy]

my skyrocketing html test file is here: https://gist.github.com/Elexy/9217076
Doesn't seem to do that anymore because i get an error:"Uncaught TypeError: Cannot call method 'replaceChild' of null" on line 664: node.parentNode.replaceChild(newNode, node);
I'm tryning to diff the document.documentElement and that has no parent ;-)

Yes I need to transport textarea content as wel, yes. I saw that code, nice!

Just to be clear, i have a working solution, that uses simple text diffs on the html string but this opens up more possibilities because the patches are readable and therefor parse-able.

I'm not sure all my requirements should be integrated in a dom diff library. On the other hand, for performance reasons, it makes sense to go though the DOM only once and handle the property -> attribute conversion. Applying the diff after creating has a performance penalty, no?

...and more ponderings...

[johanneswilm]

hey @Elexy : I would appreciate it if you could test all this and give me all the results. The file you link to doesn't include the diffDOM library, so I don't know how you are running that.

So adding that to start with, I get this:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
                "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html class="htmlclass" leuke="attribute">
<head class="headclass">
        <title>page 1</title>
  <script src="../diffDOM.js"></script>
</head>
<body>
<h1>Pagina spandiv</h1>

<h2>SPAN with input</h2>
<span>you can read this
                <input type="text" id="aircomonteur (mbo-1)" value="test"/>
</span>
<h2>confidential SPAN with input</h2>
<span class="confidential">this is secret
                <input type="text" id="aircomonteur (mbo-1)"/>
</span>
<h2>SPAN with confidential input</h2>
<span>this is not secret
                <input class="confidential" type="text" id="aircomonteur (mbo-1)"/>
</span>
<h2>confidential SPAN</h2>
<span class="confidential">secret span</span>
</body>
</html>

Then I open the page and in the console I run:

> dd=new diffDOM();
> dd.diff(document.documentElement,document.documentElement)
[]

That's right, thereshouldn't be any difference between the two. Then I do:

> a=document.documentElement.cloneNode(true);
> dd.diff(a,document.documentElement);
[]

Still correct.

Now I manually edit the first text field and remove the "1" and run the above again:

dd.diff(a,document.documentElement)
[Diff]

Bingo! Everything works as it should.

If it doesn't work for you, please send me the file with the exact javascript program you are trying to execute.

[johanneswilm]

I noticed you added a diff to 793500b . correct? You deleted it again?

[johanneswilm]

As a continuation of the above:

> thediff = dd.diff(a,document.documentElement)
[Diff]
> dd.apply(a,thediff)
true
> thediff=dd.diff(a,document.documentElement)
[]

So after applying the diff between a and document.documentElement to a, the two elements are once again the exact same.

[johanneswilm]

@Elexy: I found a bug when dealing with input fields that are empty. This happened to you when you edited the second input field. The first one worked fine.

That has now been fixed. Also, this should deal with radio boxes and selected options. But please try it out.

[johanneswilm]

@Elexy @marcelklehr : I have now also shrunk the size of diffs when in production mode by using numbers for field names. I think bug fixes and performance enhancements are the main thing missing now.
Please let me know if this works for forms now. If it does, we should close this issue.

[Elexy]

ok, will test

[Elexy]

I'm testing this inside my application and i keep getting 'aw snap' without giving me an opportunity to debug.... will report back.

[marcelklehr]

will test asap..

[Elexy]

Ok, tested inside my app, and working great. :-)

I've forked your repo, because I'd like to add automated testing with karma to it. You like the idea?

[johanneswilm]

I am not usually in favor of over-complicating things. But if you make sure to integrate the two already existing tests and it's simple and clear how to add more tests, we can go that way. Check out the random test page first and see if you can recreate that with that testing framework of yours. I am closing this issue now. Please open another issue for other things.

[johanneswilm]

@Elexy @marcelklehr I have now added tests for form elements (see above commit).

[marcelklehr]

I think it's a little ugly to have separate operations for the checked, selected and value properties.
Since, "most of the time you'll want to deal with the properties anyway" instead of the attributes, I think refactoring to always use the properties would make checked, selected and value blend in nicely with the other props/attribs.

also, see https://stackoverflow.com/questions/5874652/prop-vs-attr#answer-5884994

I'm not fussed about this though, since it works anyway, obviously.

[johanneswilm]

Hey,
this has already been fixed to work for your specific case in the most correct way possible, right? I fear turning them into attributes will make the code rather unreadable/spaghetti code. If you feel like working more with the library, maybe testing it on other cases could reveal cases in which it doesn't work.

As you mentioned correctly, that issue mentioned in the link is not relevant.