Opened 12 years ago
Closed 11 years ago
#9982 closed New Feature (fixed)
Introduce fake-selections
Reported by: | Frederico Caldeira Knabben | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.3 Beta |
Component: | Core : Selection | Version: | |
Keywords: | Cc: | wim.leers@… |
Description
A fake-selection is a "virtual" element selection that happens in the editor. No UI artifacts are rendered for such selection and the current native selection is remove. Still, the CKEditor selection object works properly, having all its properties return values that match the fake-selected element.
- Any kind of element can be fake-selected.
- Fake-selection fires "selectionChange".
- Native selection must be managed so a fake-selection is kept if no other selection actions are taken from the user.
- Fake-selection is dismissed when the user makes any other selection action (e.g. click in the document).
- Fake-selection must be "bookmarkable" just like a normal selection.
- Fake-selection must participate on Undo/Redo just like a normal selection.
- Fake-selection must be restored on editor Blur/Focus just like a normal selection.
Attachments (2)
Change History (17)
comment:1 Changed 12 years ago by
Owner: | set to Frederico Caldeira Knabben |
---|---|
Status: | new → assigned |
comment:2 Changed 12 years ago by
comment:3 follow-up: 4 Changed 12 years ago by
Cc: | wim.leers@… added |
---|
What does "bookmarkable" mean in the description?
Can you clarify why precisely this (i.e. fake selection) is required by #9764?
comment:4 Changed 12 years ago by
Replying to Wim Leers:
What does "bookmarkable" mean in the description?
It' CKEditor-ish... selections can be "bookmarked" so they can be reselected later. For example, you create a bookmark, make several DOM manipulations in the editor and then select the bookmark to have the previous selection in place.
Can you clarify why precisely this (i.e. fake selection) is required by #9764?
We have to create a fake-selection when we select a Widget. At that point, not native browser selection exists in the editor until the user clicks on other parts of the document. This ticket handles this requirement, bringing the fake-selection feature and making it work just like a normal selection, in the selection API point of view.
comment:5 Changed 12 years ago by
Thanks for the clarification. I don't pretend to fully understand what you mean, but it definitely helps — thanks!
comment:6 Changed 12 years ago by
Milestone: | CKEditor 4.1 → CKEditor 4.2 |
---|
Along with #9764 this ticket is postponed to 4.2.
comment:10 Changed 12 years ago by
Owner: | changed from Frederico Caldeira Knabben to Piotrek Koszuliński |
---|
Time for little update. Ok... not so little :P
editor.getSelection
returns null if not in wysiwyg mode - this should prevent from editor firingselectionChange
when switching to source mode and is just right.- I introduced
selection.rev
property which means selection revision. This property is incremented every time new selection instance is created and when selection is changed byselect*/reset
methods. - I introduced selection cloning. Clone has the same rev and contains copy of original selection's object. This feature is useful only in one case - it makes sense only together with selection locking in case when we don't want to lock original selection. See 4.
- I reused 2. and 3. in code locking selection which is then saved on editable blur.
- I added condition that only selection which was faked (or its clone, thanks to 2.) can reset itself. Thanks to that fake selection is safer and I know that algorithms work correctly, because otherwise errors are logged on console (waiting for #9786 to enhance that). This condition already proved its usefulness, because I found few issues.
- This is the most important change - real selection is hidden by moving it to hidden container, rather than by removing all ranges from it. This fixed instability of previous solution, because there's a real selection somewhere in editable, so browsers are not doing some crazy things. It is possible to check whether real selection is still in hidden container by calling
editor.getSelection( true ).isHidden()
. When, because of user actions, real selection is moved from hidden container, then fake selection is reset - similarly to the previous solution. - Since selection is hidden, but still in editable, we have to prevent all default actions triggered by keystrokes (like arrows, typing, etc.) and reimplement chosen ones. I added convenient way for that, by adding new argument to sel.fake() which is a fake selection definition accepting keystroke handlers. By default - when there's no handler for a specific keystroke, then that keystroke is completely blocked (preventDefault, stopPropagation and cancel). Note: I've just realised that I implemented key handlers, not keystroke handlers - will fix this asap.
- Side thing -
editable.attachListener
returns listener instance, so it is possible to remove it manually.
I attached two samples with very simple implementation of "click to make a fake selection". It's surprising that just few lines of code create such an experience. Note: check the console.
Known issue - on IE9 it is not possible to make a fake selection on image, but most likely it's easy to fix. Not important for sample though.
Not yet implemented & tested - undo manager integration. Since hidden container is appended to editable it definitely creates snapshot.
comment:11 Changed 12 years ago by
The current code looks pretty consistent and works well for me on Chrome, FF and IE9 (except for image, as you described).
I see that all keystrokes are blocked... I have mixed feelings about it, because we may be hardly able to understand which keystrokes should not be blocked. Wouldn't it be better to just block those that we want to have custom handlers assigned to (like the directional keystrokes, del/backspace, etc.)?
The isHidden method doesn't look like a good API. It's an internal tool, so it should be kept internal, if possible.
comment:12 Changed 12 years ago by
I see that all keystrokes are blocked... I have mixed feelings about it, because we may be hardly able to understand which keystrokes should not be blocked. Wouldn't it be better to just block those that we want to have custom handlers assigned to (like the directional keystrokes, del/backspace, etc.)?
I've got doubts regarding that too. The thing is that hidden element is placed inside editable, so undo manager knows about it. Thus, not only we have to block keystrokes that can destroy that hidden element (delete, backspace) or move selection out from it (arrows, home, end, page down/up), but also all that actually types something (all character keys, enter, ctrl+v, tab in some cases) ;| So nearly all.
There are two other ways to handle this:
- Removing hidden element before taking snapshot and restoring it later (including restoring selection) - awful hack which would make troubles.
- Removing contents of hidden element using regexp. It can be based on some comments inside hidden element. E.g. initial content:
<!--cke_hidden_selection_element_s-->[ ]<!--cke_hidden_selection_element_e-->
But still - typing, pasting, enter may have so much side effects that we will not be able to control that. Things to worry are - random scrolling, hidden element position (it's tricky to actually hide it on IE), splitting hidden element, etc.
So, I'd not spend more time on this now. Especially, that we don't see final result, when some keystrokes have custom handlers.
The isHidden method doesn't look like a good API. It's an internal tool, so it should be kept internal, if possible.
Initially I made it completely private, but I needed it in tests and those samples, so it became a method. I also think that it should not be public, so I'll add a @private tag.
Changed 11 years ago by
Attachment: | fakesel.html added |
---|
Changed 11 years ago by
Attachment: | fakesel_inline.html added |
---|
comment:13 Changed 11 years ago by
Branches t/9982 are no longer maintained - there was too strong cross-dependency between t/9764 and t/9982, so I'm currently working on t/9764 only.
comment:14 Changed 11 years ago by
Milestone: | CKEditor 4.2 → CKEditor 4.3 |
---|
Pushed t/9982 @cksource and @tests. It's a first version for this feature. All tests passing, except on IE.
Before spending time on the IE issues research, we should put this branch in action with #9764, making it work with Chrome and Firefox. Therefore, I'm asking for a first review, not taking in consideration IE issues. It should be feature complete in with other browsers though.