Different behaviour on delete and backspace for emptied spans?

Friedrich W. H. Kossebau

26 Jun 2013 26 Jun '13

1:56 p.m.

Hi, when testing if there will be any regressions on always normalizing* the position and length in OpRemoveText I found that there is different behaviour if in the following structure the char is removed by a backspace or a delete: text:p text:spanA Currently these are the results: ... on a backspace from behind the "A" text:ptext:span (ignore the cursor position, different topic, handled in separate email) ... on a delete from before the "A" text:p I would have expected to get the same results in both cases. Playing with LO I found that there the span is kept in both cases (tested by creating an empty document, inserting an "A", marking it and directly formatting it to bold, removing the A in either ways again and saving as .fodt and inspecting with a text editor, while also seeing that with the empty document the Bold is toggled and any inserted chars are ending up in the span, being formatted with Bold). * normalizing by doing this inside the execute method and restoring original values afterwards: if (length < 0) { position = position + length; length = -length; } What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border? Cheers Friedrich -- Friedrich W. H. Kossebau // KO GmbH http://kogmbh.com/legal/

Show replies by date

Philip Peitsch

27 Jun 27 Jun

3:21 a.m.

On 26/06/2013, at 11:56 PM, Friedrich W. H. Kossebau wrote:

...

Hi,

when testing if there will be any regressions on always normalizing* the position and length in OpRemoveText I found that there is different behaviour if in the following structure the char is removed by a backspace or a delete:

text:p text:spanA

Currently these are the results:

... on a backspace from behind the "A"

text:ptext:span

(ignore the cursor position, different topic, handled in separate email)

... on a delete from before the "A"

text:p

I would have expected to get the same results in both cases.

+1 for this. This is definitely a side-effect, not an intended thing

...

Playing with LO I found that there the span is kept in both cases (tested by creating an empty document, inserting an "A", marking it and directly formatting it to bold, removing the A in either ways again and saving as .fodt and inspecting with a text editor, while also seeing that with the empty document the Bold is toggled and any inserted chars are ending up in the span, being formatted with Bold). … [snip] What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border?

Ideally the deletion should only happen once the range crosses the node's boundary. This is going to be tricky however as currently OpRemoveText is REALLY text oriented, and really will need to be re-written to be able to handle the (imo correct) behaviour you're proposing. We'll need to rewrite it anyways, as a the moment it is unable to remove images, tables, etc. and is not really terribly reliable on complex documents. I can also see no easy way to adapt the existing code easily to accommodate this, as the whole operation is driven by text neighbourhoods. Hence the recent tab deletion patch, it was easier to ADD text for the operation to remove. (No criticism intended of the original code btw… it has served it's purpose well so far :-), I just think we're now rapidly out-growing it!) I have some ideas about using position iterators and ranges, but similar to a top-secret military-funded research project, I currently have no functional implementations ;-). The basic intent is this becomes OpRemoveRange instead, and is focused primarily on removing "valid" ODT positions in the supplied range.

Aditya Bhatt

9:03 a.m.

New subject: Different behaviour on delete and backspace for emptied spans?

On 27-Jun-2013, at 5:21 AM, Philip Peitsch wrote:

...

On 26/06/2013, at 11:56 PM, Friedrich W. H. Kossebau wrote:

...
I would have expected to get the same results in both cases.

+1 for this. This is definitely a side-effect, not an intended thing

I agree.

...

...
Playing with LO I found that there the span is kept in both cases (tested by creating an empty document, inserting an "A", marking it and directly formatting it to bold, removing the A in either ways again and saving as .fodt and inspecting with a text editor, while also seeing that with the empty document the Bold is toggled and any inserted chars are ending up in the span, being formatted with Bold). … [snip] What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border?

Ideally the deletion should only happen once the range crosses the node's boundary. This is going to be tricky however as currently OpRemoveText is REALLY text oriented, and really will need to be re-written to be able to handle the (imo correct) behaviour you're proposing.

We'll need to rewrite it anyways, as a the moment it is unable to remove images, tables, etc. and is not really terribly reliable on complex documents. I can also see no easy way to adapt the existing code easily to accommodate this, as the whole operation is driven by text neighbourhoods. Hence the recent tab deletion patch, it was easier to ADD text for the operation to remove. (No criticism intended of the original code btw… it has served it's purpose well so far :-), I just think we're now rapidly out-growing it!)

I agree that OpRemoveText is very text oriented, and the neighbourhood approach might be obsolete now that we should be able to access iterators and filters in the operation itself. I don't mind trashing the neighbourhood code once an approach that passes all tests and has at least all the capabilities of the current technique is written.

...

I have some ideas about using position iterators and ranges, but similar to a top-secret military-funded research project, I currently have no functional implementations ;-). The basic intent is this becomes OpRemoveRange instead, and is focused primarily on removing "valid" ODT positions in the supplied range.

Makes sense. But let's wait for Jos's input on this. i.e., till monday, before an OpRemoveRange is made. Tobias: Any comments?

...

_______________________________________________ WebODF mailing list WebODF@nlnet.nl https://open.nlnet.nl/mailman/listinfo/webodf

Jos van den Oever

3 Jul 3 Jul

1:06 p.m.

New subject: Different behaviour on delete and backspace for emptied spans?

On 06/27/13 11:03, Aditya Bhatt wrote:

...

On 27-Jun-2013, at 5:21 AM, Philip Peitsch wrote:

...
On 26/06/2013, at 11:56 PM, Friedrich W. H. Kossebau wrote:

...
I would have expected to get the same results in both cases.

+1 for this. This is definitely a side-effect, not an intended thing

I agree.

...
...
Playing with LO I found that there the span is kept in both cases (tested by creating an empty document, inserting an "A", marking it and directly formatting it to bold, removing the A in either ways again and saving as .fodt and inspecting with a text editor, while also seeing that with the empty document the Bold is toggled and any inserted chars are ending up in the span, being formatted with Bold). … [snip] What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border?

Ideally the deletion should only happen once the range crosses the node's boundary. This is going to be tricky however as currently OpRemoveText is REALLY text oriented, and really will need to be re-written to be able to handle the (imo correct) behaviour you're proposing.

We'll need to rewrite it anyways, as a the moment it is unable to remove images, tables, etc. and is not really terribly reliable on complex documents. I can also see no easy way to adapt the existing code easily to accommodate this, as the whole operation is driven by text neighbourhoods. Hence the recent tab deletion patch, it was easier to ADD text for the operation to remove. (No criticism intended of the original code btw… it has served it's purpose well so far :-), I just think we're now rapidly out-growing it!)

I agree that OpRemoveText is very text oriented, and the neighbourhood approach might be obsolete now that we should be able to access iterators and filters in the operation itself. I don't mind trashing the neighbourhood code once an approach that passes all tests and has at least all the capabilities of the current technique is written.

...
I have some ideas about using position iterators and ranges, but similar to a top-secret military-funded research project, I currently have no functional implementations ;-). The basic intent is this becomes OpRemoveRange instead, and is focused primarily on removing "valid" ODT positions in the supplied range.

Makes sense. But let's wait for Jos's input on this. i.e., till monday, before an OpRemoveRange is made.

In the meantime a patch landed which removes the backwards delete and now only forward ones are remaining. Friedrich asked: "What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border?" Answer: empty spans should not remain because the cursor cannot enter them anyway. To start typing styled text, the ui should remember the style associated with the cursor and apply it to the just entered text. So InsertText followed by ApplyStyle. Cheers, Jos

4214

Age (days ago)

4221

Last active (days ago)

List overview

Download

3 comments

4 participants

participants (4)

Aditya Bhatt
Friedrich W. H. Kossebau
Jos van den Oever
Philip Peitsch