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

[CoEditor] HTML control characters are being displayed #167

Closed
vinomaster opened this issue Mar 7, 2012 · 7 comments
Closed

[CoEditor] HTML control characters are being displayed #167

vinomaster opened this issue Mar 7, 2012 · 7 comments
Assignees
Milestone

Comments

@vinomaster
Copy link
Member

This bug is from iTel Companies

User1: Safari
User2:iPBS (webkit)

Scenario: User1 starts coweb session using Document Editor. User1 types characters, some characters displayed in User2 instance of editor, HTML control characters embedded with text. If User2 is typing, then User1 sees HTML control characters.
Synch between Safari and Safari or Chrome has no such issues

@ghost ghost assigned bitpshr Mar 7, 2012
@vinomaster
Copy link
Member Author

@bitpshr
Copy link

bitpshr commented May 23, 2012

Will look at this over the course of this week.

@ccotter
Copy link
Member

ccotter commented Jun 6, 2012

I've been able to reproduce the exact sequence described above with [https://github.com/opencoweb/cowebx/commit/f870ddc2c3076ffe86ddcb968df2a6177b7aae49] running locally, although not deterministically; there are other use cases that cause control characters to display, but I cannot find a use case that always reproduces the issue. I will investigate the cause of this.

@ghost ghost assigned ccotter Jun 6, 2012
@ccotter
Copy link
Member

ccotter commented Jun 6, 2012

Here is a list of issues discovered and supposedly fixed by my code updates (i.e. resolved):

  • This use case will almost always cause the bug to manifest: User1 types very quickly within the sync timeout (or just copy and pastes) an ampersand and semicolon, &; The actual innerHTML that is send is &; When User2's javascript runs hasIncompleteTags, the ampersand count will be 1, but semicolon count will be 2. Thus, User2's screen will never be updated since it will continually think the HTML message send from the server is not complete.

  • This use case is difficult to replicate since it relies on some subtle timing of the server sending events. Suppose User1 deletes some text, and that the deletions occur over a range of HTML markup. If the delete message is broken up and send as two separate messages from the server to User2, then User2 might leave the HTML in an incomplete state:

    User1 changes innerHTML from <div><br></div><div><br></div> to <div><br></div> (15 character deletion), but the server sends two separate messages: delete 1 character, then delete 14 characters: User2 first transforms the original HTML into <div><br></div>div><br></div>, but NicEdit immediately converts the dangling div> into div&gt;, then User2 receives the delete 14 character message. hasIncompleteTags is insufficient since it does not detect whether deletions will deform the HTML.

  • Another cause is rangy's saveSelection. Upon receiving remote commands to delete characters, the client:

    1. calls rangy.saveSelection
    2. performs the deletions (taking offsets into account the hidden <span> generated by rangy)
    3. sets the text area innerHTML
    4. calls rangy.restoreSelection

    Sometimes, after step 2, since the diff utility util.ld is agnostic to the arbitrary rangy code injection, rangy's <span> might have inadvertently moved inside a HTML tag. For example : ...<b><span ...></span>... becomes ...<b<span ...></span>>...

List of known issues with fixes, but not committed to github

  • There is still an issue involving Rangy's invisible markers. If the markers are at positions where characters are inserted or deleted (and maybe updated), then sometimes the markers end up inside other HTML tags and the text is out of sync. The fix is to try to re-insert the markers, and if the HTML becomes malformed, revert to the HTML without Rangy's markers (and we lose the caret position - but at least the text stays in sync).

List of known issues without fixes (i.e. non-resolved)

@bitpshr
Copy link

bitpshr commented Jun 7, 2012

When I left off, the two major issues causing these problems were:

  1. Rangy is probably not the best solution to programmatically driving the caret position inside a contentEditable div here. It blasts in unwanted "marker" spans, screwing with the position of ops. A solid, side-effect free method of driving the cursor in a contentEditable div needs to be implemented. Lots of talk about this around the web, not a single cross-browser solution yet.
  2. We LD the entire DOM and sync the changes across the wire. This would work fine, but due to cross-browser differences in how contentEditable content is translated into nodes, this can be problematic. Some method needs to be implemented to smooth over cross browser DOM differences within the contentEditable div.

@ccotter
Copy link
Member

ccotter commented Jun 8, 2012

I've pushed updates that appear to resolve most of the issues relating to this bug. After testing with Safari, Chrome, and Firefox separately, I did not seen any issues. I did less cross browser testing (Chrome and Firefox, for example), and I am not convinced cross browser issues will completely disappear.

In the meantime, I'll keep the above comment updated with a list of use cases that cause this bug, and the status of a fix for that use case.

Commit: opencoweb/cowebx@17c4e48

@ccotter
Copy link
Member

ccotter commented Oct 16, 2012

The specific causes mentioned in this issue have been resolved, although in general there is still a possibility for inconsistency in the coeditor, see #185.

@ccotter ccotter closed this as completed Oct 16, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants