backspace at start of line gives strange results in Webkit

[johanneswilm]

Try the following (using a Webkit browser):

1. Go to http://nytimes.github.com/ice/demo/
2. In the first demo, scroll down to "But in reality it is..."
3. Insert three new paragraphs above it. Each one of no more than a few lines
4. Put your cursor at the end of the last paragraph you create in 3.
5. Hit the Backspace key until they entire paragraph is gone and the Backspace key is at the start of the line.
6. Hit Backspace one more time.

Expected results: The cursor should delete the now empty paragraph and move to the paragraph above it where it should be placed at the very last position.

What happens: Instead the "B" of the paragraph below it is deleted (the one that goes: "But in reality...").

In Firefox this works fine.

[johanneswilm]

I have started looking at this issue. It seems to be at least in part related to #41 and actually happend both in Webkit (all the time) and Firefox (sometimes). Basically when inside two inserts that are inside oneanother (in a paragraph that has been emptied), it seems to get confused as which direction it should move (up or down). There is also an issue where the carety moved this way:

<p>Hey</p>
<p>Hey</p>
<p>|</p>

and if one now clicks backspace once, one would expect for the caret to end up at the end of the second line, however, it ends up at the start instead:

<p>Hey</p>
<p>|Hey</p>

I think a general policy has to be figured out about whether inserts can be nested or not. Currently it can, and this may make sense some time and at other times not. Take the following example

<p><insert 1>This is a completely new paragraph that someone inserted here. Therefore the entire paragraph <delete 2>should</delete 2><insert 2>will</insert 2> be highlighted</insert 1></p>

In the above example it may make sense that the second insert is nested in the first insert. The document owner will first be asked whether he accepts the insert 1, and only if he confirms will he be presented with delete 2 and insert 2.

However, imagine this case:

<p><insert 1>This is a completely new paragraph that someone inserted here. Therefore the entire paragraph will be highlighted</insert 1>|</p>

Where | is the caret. If user 2 has the caret there, hits retrun and writes another paragraph, the following code is produced:

<p><insert 1>This is a completely new paragraph that someone inserted here. Therefore the entire paragraph will be highlighted</insert 1>|</p>

<p><insert 1><insert 2>This is the second paragraph. It is only written by user 2, yet it is wrapped into an insert element of user 1.</insert 2></insert 1></p>

This second example does not look right. User 1 has nothign to do with the paragraph, so why would an insert for user 1 be connected to it.

Additionally, this nesting gives problems with this error, in both main browsers, both with and without TinyMCE.

One alternative could be to not accept inserts inside inserts ever. Another possibility could be to only accept them if user 2 writes inside a paragraph by user 1, but not when creating an entirely new paragraph. The first solutiuon would probably give cleaner results altogether.

@delambo what is your preference?

[johanneswilm]

fixed for Webkit in johanneswilm@b62ecc6

The behavior in Firefox is more sane than it was before, but not entirely fixed still.

[delambo]

@johanneswilm - with my changes to master today (and my update to the ice demo), I think the problem you described in comment 1 is fixed. Can you verify please?

Let me know what you have cooking. I'm trying to get ice fixed up, especially in webkit. I was busy on a lot of projects last year, but will have more time to devote to ice this year.

[johanneswilm]

@delambo Hey, I've merged your changes, this particular issue has not been fixed in the ICE demo you have running, but is working in my current master branch.

There is still something wrong with selection deletion of the content a user has created himself, both in my version and in yours, though somewhat differently:

In your version, if you select a longer part of text, which includes some subelements that the current user has authored, and click 'backspace' these subelements are deleted. However, if you select only parts or an entire paragraph the current user has written, and hit 'backspace', then it is only crossed out.

[johanneswilm]

the above error exists both in Webkit and in Firefox.

[johanneswilm]

In my version there is something more serious happening when deleting a paragraph written b7y the current user + some additional contents. I will look at that tomorrow.

[delambo]

@johanneswilm - I'm still not able to replicate in the online demo. Can you break down step (3). For instance, where is the cursor when you start creating new paragraphs (hit enter/return) and what do you enter into the paragraphs. Thanks.

[johanneswilm]

ah, you're right. the cursor does not move strangely any more. But a new bug has come out of this -- the paragraph before it ends with three times <insert><br></insert> if I create three new paragraphs and delete them with backspace until thye are gone again. This was fixed in my branch a while back.

[delambo]

I would prefer to handle/prevent tags from getting split into new paragraphs by attacking the problem upstream in the _handleEnter function - basically, stop it before it happens. I added the following inside of the existing ice.js _handleEnter, which seems to work in FF, but is failing in Chrome:

_handleEnter: function() {
        var range = this.getCurrentRange();
        if (!range.collapsed) this.deleteContents();

        // If the cursor is at the end of an insert, then move the cursor
        // outside so that the browser doesn't split the insert into the
        // new paragraph that is created.
        var insert = this.getIceNode(range.startContainer, 'insertType');
        while (insert && range.startContainer.data.length === range.startOffset) {
            range.setStartAfter(insert);
            insert = this.getIceNode(range.startContainer, 'insertType');
            this.selection.addRange(range);
        }

        return true;
    }

I'm going to try to massage it for Chrome - may even submit a bug request because Chrome doesn't have this behavior for a and other tags.

What do you think?

[johanneswilm]

I completely agree. This is a much cleaner way of doing it. If you do that,
then I'll pull it into my branch. I may have to do some changes to it as my
branch supports lists, headlines, etc. and I don't know if all of those
work the same as concerns of using enter.
On Jan 29, 2013 12:59 PM, "Matthew DeLambo" notifications@github.com
wrote:

I would prefer to handle/prevent tags from getting split into new
paragraphs by attacking the problem upstream in the _handleEnter function

• basically, stop it before it happens. I added the following inside of the
  existing ice.js _handleEnter, which seems to work in FF, but is failing
  in Chrome:

_handleEnter: function() {
var range = this.getCurrentRange();
if (!range.collapsed) this.deleteContents();

    // If the cursor is at the end of an insert, then move the cursor
    // outside so that the browser doesn't split the insert into the
    // new paragraph that is created.
    var insert = this.getIceNode(range.startContainer, 'insertType');
    while (insert && range.startContainer.data.length === range.startOffset) {
        range.setStartAfter(insert);
        insert = this.getIceNode(range.startContainer, 'insertType');
        this.selection.addRange(range);
    }

    return true;
}

I'm going to try to massage it for Chrome - may even submit a bug request
because Chrome doesn't have this behavior for a and other tags.

What do you think?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/40#issuecomment-12841508.

[delambo]

This is proving to be really hard in Webkit. Still working on getting this right.

[delambo]

Since the first problem is fixed, I'm going to open a new issue with the second problem.