Open core-ai-bot opened 3 years ago
Comment by njx Friday Aug 02, 2013 at 22:40 GMT
Hmmm, that's interesting. In addition to the fact that we aren't dealing with comments properly, I think there might be an issue here where the browser deliberately ignores/removes whitespace in certain cases where it's spec'ed to be non-significant (e.g. immediately inside a parent tag before the first child). If that's the case, then we probably have to do the same in order for our indices to line up.
When I was originally thinking about adding this kind of debugging, I was actually thinking of just taking all the nodes in our DOM, finding their ids, looking them up in the browser, and then seeing if the parentage and child index was the same. But I think your approach makes sense--we might want to improve the output a bit to clarify exactly where the discrepancy is (maybe dumping out the text of the relevant nodes). Does that make sense?
Comment by njx Friday Aug 02, 2013 at 22:43 GMT
Oh, I just thought of another potential problem with taking this approach...in order to do this comparison, we're parsing the browser's outerHTML with our parser. I think we actually want to compare directly to the browser's DOM structure because we want to detect cases where the browser's parser does something different from ours.
Comment by jasonsanjose Friday Aug 02, 2013 at 23:17 GMT
Closing. Will change comparison to compare DOM nodes instead of HTML string.
Comment by dangoor Saturday Aug 03, 2013 at 01:21 GMT
You may have already discussed this, but it seems like one possibility is to build a SimpleDOM based on the browser DOM and then use our differ because we'd still have the problem of "how do you find the differences?" Obviously, we don't care about running time for debugging purposes so we could do some kind of naïve and slow diff, but it seems easier just to lean on what we have (especially since the browser would have the same IDs. Should make it easy to spot the elements that don't have IDs and were likely added by the browser.)
Comment by njx Saturday Aug 03, 2013 at 01:38 GMT
Oh, that's an interesting idea (basically, take the browser's DOM structure and create a parallel SimpleDOM, so we're still getting the browser-parsed structure but can run our differ on it).
I think the more naive approach I was suggesting above (which might be what Jason is working on now) is probably fine, though--it will show us all the specific node-by-node differences and will be fairly simple to implement. I actually think the node-by-node differences might be a little easier to read than the output of our differ for debugging purposes.
Comment by dangoor Saturday Aug 03, 2013 at 01:54 GMT
On my first read, I didn't follow exactly what your proposed diff would be like. I understand what you mean now, and I agree that is simple to implement. I'm not certain that it would be easier to read. For example, insertion of a node would cause everything that follows it to have an unexpected child index. So, the output could get really verbose. And, as soon as you try to optimize that output, you're writing a diff algorithm :)
There are two things we don't catch if we just go through all of our IDs:
At some point, I suspect we'll implement the "move" operation which will also make a big difference in the size/readability of certain diffs.
I think there's a command line tool available that implements the BULD algorithm. If we really wanted an independent comparison, we could dump the browser's HTML and convert our SimpleDOM to text and then manually run that, but I think an in-Brackets tool would be more useful (and possibly something we could apply to unit tests.)
Comment by njx Saturday Aug 03, 2013 at 01:58 GMT
That's a good point.@
jasonsanjose - if you're far enough along with my previous suggestion that it's close to working, I think we should just go with that for now (don't want to deep-end too much on optimizing what's basically a debugging tool) and we can improve it later. But if you haven't started on it yet, Kevin's suggestion might be better.
Comment by jasonsanjose Monday Aug 05, 2013 at 16:24 GMT
I haven't started on it yet. I like@
dangoor's suggestion. I'll tackle that this afternoon.
Issue by jasonsanjose Friday Aug 02, 2013 at 22:18 GMT Originally opened as https://github.com/adobe/brackets/pull/4643
@
njx@
dangoorRuntime debug mode to compare in-memory DOM with in-browser DOM.
In
HTMLDocument.onChange
add a conditional breakpointthis._debug=true,false
to enable debugging.Here's some example output when pasting in a new tag. I'm using the getting started content pasting the first
h2
immediately below itself on line 15:Some things to note:
outerHTML
and treating it as the "new" tree. The editor state (after applying the doc changes) is used as the "old" tree.lost-whitespace
below)elementInsert
delta is actually the HTML element highlight we add viaRemoteFunctions
h2
tag isn't inserted as expected (seepaste-result
)So...there's still some noise that we have to filter out to make this a friendlier experience. However, we might have legitimate issues here like handling comments.
Also
jasonsanjose included the following code: https://github.com/adobe/brackets/pull/4643/commits