[CLOSED] Fix: Move Line Up/Down collapses inline editor when moving past the start/end

[core-ai-bot]

Issue by TomMalbran
Saturday Dec 22, 2012 at 00:11 GMT
Originally opened as adobe/brackets#2431

---

This is a possible fix for the issue adobe/brackets#1933.

When moving a line down on an inline editor, the problem seems to be moving the line to the last line and not moving the last line beyond the inline widget. But moving the line beyond the inline widgets breaks getLastVisibleLine() which starts giving 1 line less than it should and then breaking the move line up.

So this change doesn't move down in an inline editor when trying to move the line to the last visible line or when moving the last visible line down.

---

TomMalbran included the following code: https://github.com/adobe/brackets/pull/2431/commits

[core-ai-bot]

Comment by jasonsanjose
Thursday Jan 03, 2013 at 15:06 GMT

---

Reviewing

[core-ai-bot]

Comment by jasonsanjose
Thursday Jan 03, 2013 at 22:48 GMT

---

Tried out your fix and I'm seeing a few issues. You should be able to see them as well in the getting started project. Open an inline editor on line 14 on the h2. This opens main.css lines 19-21. Reset the file after each bug described.

• Attempt to move line 20 down. Result: Does not move.
• Attempt to move line 21 up. Result: Line 20 becomes line 22.

[core-ai-bot]

Comment by jasonsanjose
Friday Jan 04, 2013 at 18:52 GMT

---

@TomMalbran Looks like this pull request was submitted right before our winter break. Just pinging you again in case you've been on vacation. Thanks for the contribution.

[core-ai-bot]

Comment by TomMalbran
Monday Jan 07, 2013 at 14:47 GMT

---

I knew it was before the vacations, but I got the idea about the fix right back then, and yes, I am on vacations and with poor internet connection. I'll be back in a few days anyway and try to fix this.

• Moving the line 20 down on a 19-21 widget closed it, so as a fix, I blocked it. There might be a better way to fix this, but not within the function.
• I didnt noticed the other issue, so the solution would be to block the moving up of the last line.

[core-ai-bot]

Comment by TomMalbran
Thursday Jan 10, 2013 at 00:15 GMT

---

I fixed the second issue mentioned.

But I cant find a way to fix the first one, since the move line up generates a document change that gets into the special case of TextRange.prototype._applySingleChangeToRange which ultimately removes the link to the inline editor and closing it. I am not sure if there is a way to avoid this, but anyway, in the bug issue it was expected that the text shouldn't move if moving it would cause the inline editor to close, so this fix does this, even thought the line that causes the editor to close is not the last one as mentioned but the second to last, which is the one that I blocked from moving down.

[core-ai-bot]

Comment by jasonsanjose
Saturday Jan 19, 2013 at 00:26 GMT

---

I wasn't able to get this tested in time for sprint 19. We'll take a look at this again for sprint 20.

[core-ai-bot]

Comment by jasonsanjose
Friday Feb 15, 2013 at 23:18 GMT

---

@TomMalbran more apologies for being out sick the last week. Do you have time to try out your fix now that CodeMirror 3 has landed?

[core-ai-bot]

Comment by jasonsanjose
Friday Feb 15, 2013 at 23:35 GMT

---

Actually, I had some time to try this out with cmv3. I'm still seeing some issues.

1. Open inline editor on line 51 in getting started (samp tag)
2. Move line 26 up
3. Result below:

samp
{
    display: none;

    /* hide <samp> from the browser so we can show cool features in Edge Code */}

[core-ai-bot]

Comment by TomMalbran
Friday Feb 15, 2013 at 23:39 GMT

---

Yes I just tested it on the cmv3 and found the same bug. I am trying to fix that right now.

[core-ai-bot]

Comment by TomMalbran
Friday Feb 15, 2013 at 23:45 GMT

---

That didn't completely fixed the problem. Trying again.

[core-ai-bot]

Comment by TomMalbran
Friday Feb 15, 2013 at 23:57 GMT

---

@jasonsanjose Now it should be properly fixed. I tested on several inline and main editors and it worked on all of them.

[core-ai-bot]

Comment by jasonsanjose
Tuesday Feb 19, 2013 at 17:31 GMT

---

Looks good. Merging.