adobe/brackets — pull request #4317 by njx (2013-06-22 00:04:33+00:00)
Title: Hooking up more edits and invalid/valid handling to the UI

Might want to review the commits individually. The code gets worse and worse in later commits :) But most of the gross code is in RemoteAgent/HTMLDocument, and that stuff is all throwaway/easy to recreate more nicely.

The commit names should be pretty self-explanatory. Here are a few notes:

* With these changes, the UI should more or less handle all the various kinds of edits, with some caveats. Cases that work (at least in my limited testing on citrus completed):
* the things that worked before (modifying existing text, changing the src of an img attribute)
* deleting an entire text span at once
* copying an `` tag and pasting it in another location
* editing the src attribute of the `` tag that was just pasted
* deleting an `` tag
* typing new attributes in a tag (see note below on valid/invalid states)
* Cases that don't work well:
* typing an entirely new tag (see note below on valid/invalid states). But if you paste something like "" all at once, then type new attributes into it, it works.
* probably lots of other stuff :)
* All of the incremental unit tests are passing now except one (simple tag insertion), but that was failing before my changes as well (though with different symptoms). I think what's happening is that the edit list for tag insertion might not be technically incorrect, but is deleting and recreating a node that could just be left alone.
* I didn't add tests for any of my new functionality. Sorry :)
* I also fixed the bug you noted before (about old node map entries not getting deleted), and also did work to properly add/update marked ranges based on the new subtree. Note that the add/update marked range code is very inefficient because there's no quick way to find the mark for a given tag ID--we probably need to cache the marks somewhere.
* The edit descriptors need some cleanup--there's some inconsistency between the various descriptor types in terms of usage of "parentID" vs. "tagID", etc. Also, we should probably tune their structure towards what will make it easiest to do the update in the browser--right now some things are easier/harder to do based on the kinds of things that are available in jQuery (e.g. it's hard to select text nodes), which is part of the reason the RemoteAgent code I added is so terrible. If we switch to a pure DOM implementation on the backend, that will make other styles of update easier/harder.
* To deal with valid/invalid states, I did basically what we talked about: made the parser (and everything upstream of it) return null if it detects a bad state anywhere; in that case, we don't do any updates until the next time the parser returns a valid DOM node, at which time we fall back to a full diff. After I added another case in the parser to detect when we seem to have unbalanced open/close tags, it now seems to catch cases of invalid attribute syntax pretty well. However, it doesn't detect the very common case where I just type "<" to start a new tag immediately before some text--the tokenizer doesn't seem to notice that eventually you run into another "<" without having closed off the first tag. That leads to some weird behavior where we create a new bogus tag and then don't recover from it later.
* If I had time, the first thing I'd do is rewrite the stuff I added to RemoteAgent to use a small library of manipulation functions on the browser side (instead of just eval-ing whole chunks of code all the time), and do any on-the-fly code gen that we need through Mustache templates. But I ran out of time :), and to the extent that it works it's probably not that important to clean it up right now.