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

Position of Caret and Selection are not updated correctly when change occurs at their current position #761

Closed
SkyAphid opened this issue Jun 30, 2018 · 11 comments
Labels

Comments

@SkyAphid
Copy link
Contributor

SkyAphid commented Jun 30, 2018

If you type into a StyleClassedTextArea, then press CTRL-Z, then begin typing again, the cursor gets reset to the start of the text.

Code:

StyleClassedTextArea textArea = new StyleClassedTextArea();
textArea.replaceText(node.getText());
textArea.setStyle("-fx-font-family: '" + textFont.getFamily() + "'; -fx-font-size: " + textFont.getSize());
textArea.setWrapText(true);
		
//Update body text
textArea.textProperty().addListener((o, oldText, newText) -> {
snip
});
		
VirtualizedScrollPane<StyleClassedTextArea> scrollPane = new VirtualizedScrollPane<StyleClassedTextArea>(textArea);
		
StackPane.setAlignment(scrollPane, Pos.CENTER);
StackPane.setMargin(scrollPane, new Insets(START_BODY_Y, 20, 20, 20));
		
getChildren().add(scrollPane);

Also, if there's no way to fix this, is there a way to just disable CTRL-Z entirely?

@JordanMartinez
Copy link
Contributor

As in (assuming | is the caret) this?

|     // start
text| // after inputting "text"
|     // CTRL-Z pressed
|text // after inputting "text" (caret should be after the second "t" but isn't)

@SkyAphid
Copy link
Contributor Author

I've recorded the problem, you can took a look here:

https://drive.google.com/open?id=1WqPV4vah2imL-w5vwkiTs06zT_3WqVeF

So yes, I think you're right. Just wanted to help clarify it.

@JordanMartinez
Copy link
Contributor

Thanks for the video. However, it'll be easier for me to debug and fix this if you submit a reproducible copy-and-paste demo like this one rather than a video showing the same thing. Could you do that instead?

@SkyAphid
Copy link
Contributor Author

SkyAphid commented Jun 30, 2018

import javafx.geometry.Insets;
import javafx.geometry.Pos;

import org.fxmisc.flowless.VirtualizedScrollPane;
import org.fxmisc.richtext.StyleClassedTextArea;

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;

public class BugFix extends Application {
    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws Exception {
        StyleClassedTextArea textArea = new StyleClassedTextArea();
        textArea.replaceText("Text");
        textArea.setWrapText(true);
        		
        //Update body text
        textArea.textProperty().addListener((o, oldText, newText) -> {
        	
        });
        		
        VirtualizedScrollPane<StyleClassedTextArea> scrollPane = new VirtualizedScrollPane<StyleClassedTextArea>(textArea);
        		
        StackPane.setAlignment(scrollPane, Pos.CENTER);
        StackPane.setMargin(scrollPane, new Insets(20, 20, 20, 20));

        final Scene scene = new Scene(scrollPane, 600, 400);
        primaryStage.setScene(scene);
        primaryStage.show();
    }
}

Here.

Edit:
To give a bit more feedback, try typing something, waiting a second, then typing again, and then press CTRL-Z. Then start typing again, that should reset the caret to the start of the text area. This test was able to reproduce the bug for me.

@SkyAphid
Copy link
Contributor Author

SkyAphid commented Jul 2, 2018

Update: I've been using RichTextFX to highlight syntax in my program. The Ctrl-Z functionality works even weirder in that case. It seems to just undo highlighting instead of actually remove text. You can reproduce the strangeness in your JavaKeywords demo.

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Jul 2, 2018

To give a bit more feedback, try typing something, waiting a second, then typing again, and then press CTRL-Z. Then start typing again, that should reset the caret to the start of the text area. This test was able to reproduce the bug for me.

Thanks for giving that additional bit. I know it's a bit pointless since I can probably infer that from your comments, but again, it just makes my job that much easier so I can focus on the issue and not on how to reproduce it.

That being said, here's why it occurs. Two or more areas can render the same document. This allows one to simulate the "split document" feature of Microsoft Word. In other words, I can use one area to see the "top" part of the document and another area to see the "bottom" part of the document. However, when one area adds/removes text to the document, the other areas' carets and selections' positions should be updated so that they still "point" to the same place as before:

| = area 1 caret
^ = area 2 caret
=== Before text is deleted ===
Some text here|
Some text^
========================
(text that gets deleted is paragraph 0's "Some text" or a removal of 9 characters
=== After text is deleted w/o update ===
 here|
Some text_ _ _ _ _ _ _ _ _ ^ // (where "_" represents 1 character
========================
This produces something like "IndexOutOfBoundsException: caret is at 
   a position that is 9 chars beyond area's length"
=== After text is deleted with update ===
 here|
Some text^
========================

So, we need to update the positions to still work. We do that by subscribing to the plainTextChanges() event stream. Whenever a text change is made, it emits the text that was added and removed at a given starting position. From that, we determine whether the position was to the right of the change and the net characters that were added/removed does not equal 0 (such as replacing "some text" with "other text"). If these checks pass, we update their corresponding positions by that amount. You can see the code here.
However, this creates the rather rare situation that you just experienced. We don't want to update the caret's position when the change occurs at the caret's position.

some text | here
// add "goes"
some text |goes here

If we did, that would put it in the wrong place. However, this is not always desirable, such as the case you're experiencing. Since the text being add is at the start of the area and the caret is at the start of the area, the code does not move the caret when it should in that situation.

I believe this could be fixed if we changed the code to move the caret (and selection) as well when both the change's position and the caret's (and selection's) position is at 0. However, I cannot think of a case off the top of my head when doing that would cause a new bug.

Update: I've been using RichTextFX to highlight syntax in my program. The Ctrl-Z functionality works even weirder in that case. It seems to just undo highlighting instead of actually remove text. You can reproduce the strangeness in your JavaKeywords demo.

Oh, whoops! That's easy to explain. The area's default UndoManager undoes rich text changes (e.g. any change to the text, be it just text changes or style changes or style and text changes). So, that can be resolved by using:

// plainUndoManager, or whatever that static method is called
UndoManager<PlainTextChange> um = UndoUtils.plainUndoManager(area);
area.setUndoManager(um);

Please open a new issue so we can track that separately from this one.

@JordanMartinez
Copy link
Contributor

I believe this could be fixed if we changed the code to move the caret (and selection) as well when both the change's position and the caret's (and selection's) position is at 0. However, I cannot think of a case off the top of my head when doing that would cause a new bug.

Actually, I wonder whether this is just a bug. Even if one typed text when the position wasn't at 0, deleted it, and hit CTRL+Z, the bug would still occur:

Some | // initial
Some text| // typed
Some | // CTRL+Z
Some |text // CTRL+Y

If so, then this line should be if (indexOfChange <= finalPosition) {

@SkyAphid
Copy link
Contributor Author

SkyAphid commented Jul 2, 2018

Please open a new issue so we can track that separately from this one.

Sure, when I get a moment to make a reproducible test I will. In the meantime, I'll try your solution first and see if it works.

@SkyAphid
Copy link
Contributor Author

SkyAphid commented Jul 3, 2018

// plainUndoManager, or whatever that static method is called
UndoManager<PlainTextChange> um = UndoUtils.plainUndoManager(area);
area.setUndoManager(um);

This doesn't appear to be compatible with InlineCssTextArea. Is there an alternative version that works with that particular implementation?

EDIT: Hold on, I'll actually go ahead and make the new issue since this is still giving me trouble.

@JordanMartinez JordanMartinez changed the title StyleClassedTextArea: Ctrl-Z resets cursor position Position of Caret and Selection are not updated correctly when change occurs at their current position Jul 3, 2018
@JordanMartinez
Copy link
Contributor

You can use the snapshot version right now (see ReadMe) as it includes this fix.

@thestrox
Copy link

I am able to replicate this if you type into a StyleClassedTextArea, then press CTRL-Z, then begin typing again, the cursor gets reset to the start of the text in latest version (0.9.1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants