Changes between Version 1 and Version 2 of Ticket #12354, comment 22


Ignore:
Timestamp:
Sep 8, 2014, 3:33:16 PM (5 years ago)
Author:
Artur Delura
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #12354, comment 22

    v1 v2  
    1 Replying to [comment:21 Reinmar]:
    2 > But if it's unreliable, why not fixing it? It cannot be fixed? Or a fix will be too vast? You still didn't prove that we need the stack - you only showed that something is wrong with the current implementation, but not even what.
    3 Problem: We need to make snapshot each X characters inputs when holding key.
     1Current implementation of {{{UndoManager}}} does not take into consideration that there is possibility to keep key pressed which cause fire {{{input}}} event few times in a row. {{{inputFired}}}  value is incremented on each {{{input}}} event. On {{{keyup}}} event it simply decrement {{{inputFired}}} value blindly assumed that there was only one {{{input}}} event in a row. For example: User press and hold "a" key for a period of time what ends up with added a character 13 times in editor and {{{inputFired}}} have now value 13. After relesing "a" key {{{inputFired}}} is decremented by one, which result {{{inputFired}}} 12. That is unexpected behaviour, we should end up with value set to 0. To fix this issue we need mechanism which register how many {{{input}}} events was registered in a row for same {{{keyCode}}}. As a rescue comes {{{KeyEventsStack}}} which register how many {{{input}}} events was called for each {{{keyCode}}}.
    42
    5 Solution: When 25 inputs recorded make snapshot.
     3'''Goal:''' Get rid of {{{KeyEventsStack}}}.
    64
    7 Question: Do we have information about recorded inputs?
     5Instead of making {{{KeyEventsStack}}} we could also introduce fields called like {{{lastKeyCode}}} and {{{lastKeyCodeInputs}}} in {{{NativeEditingHandler}}}. Which will be set:
     6{{{lastKeyCode}}} - set key code in {{{onKeyDown}}} method.
     7{{{lastKeyCodeInputs}}} - reset to 0 when current key code is different than last one, increment by one. This will be happened in {{{onInput}}} method.
    88
    9 Anwser: Yes. It's {{{inputType}}} field in {{{NativeEditingHandler}}}.
     9'''Problem:''' In {{{onInput}}} method we don’t know which {{{keycode}}} was last one.
    1010
    11 Question: Is this field reset when X characters exceeded?
     11'''Solution:''' {{{KeyEventsStack}}} keep track of that. But that’s conflict of interest.
    1212
    13 Answer: No.
     13'''Another solution:''' We can reset {{{lastKeyCodeInputs}}} to 0 in {{{onKeyDown}}} method when current {{{keyCode}}} is different than last one.
    1414
    15 Question: Can we fix that?
     15When I get deeper into this I see that {{{KeyEventsStack}}} is not ideal yet. First of all it does not handle {{{input}}} or {{{drop}}} events. Secondly it lost registered input for {{{keyCode}}} when key is up (problem can appear when holding multiple keys). Last but not least it does not sums up {{{input}}} events for all pressed keys (but that’s just one method to write).
    1616
    17 '''Answer: Fixing that means that we want to change behaviour of current implementation, but on this implementation rely various parts of code. So fixing this can be error prone, and we decided to touch current implementation as little as possible, to avoid regresstions.'''
    18 
    19 Sulution: Implement parallel solution, which is key stack.
    20 
    21 Just almost open sourced my mind ^^
     17So yes, we can get rid of {{{KeyEventsStack}}} but, it will make code more complicated, because we introduce more fields, and logic in {{{NativeEditingHandler}}}. It does not separate logic from model. {{{KeyEventsStack}}} model keyboard situation in more sophisticated way. We need less fields in model and more methods instead, to make things clear.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy