82813 - Whitespace handling needs to be savvy to <pre>

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P1)

Description

[Joe Francis]

WS handling code that I landed for moz091 does not check for <pre>.  I need to
add this in.  WS handling is very different for pre cases.  I'll attachsome test
cases later on.

Comment 1

[rubydoo123]

Joe, moving this to 1.0, it seems like there is a lot fo detailed work that
needs to take place.

Comment 2

[Joe Francis]

*** Bug 111974 has been marked as a duplicate of this bug. ***

Comment 3

[[e-mail]]

There are four testcases in the duplicate 111974 but I have since found a fifth.
If you type extra spaces at the end of a <pre> line then when you try to delete
them you end up deleting the text instead...

Comment 4

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 5

[Joe Francis]

pulling back to 098

Comment 6

[Joe Francis]

pushing off 098 to 099

Comment 7

[Kathleen :Brade]

marking EDITORBASE+ per meeting

Comment 8

[Joe Francis]

Attachment: diffs for nsWSRunObject.h

Comment 9

[Joe Francis]

Attachment: diffs for nsWSRunObject.cpp

this is a limited fix.	You will see uneven preservation of whitespace when you
merge two blocks, one a pre and one not.  However, to fix that I have to do
more than just make the current code smarter - I will have to potentially scan
through the entire block rather just it's boundaries.  To see this, consider
backspacing at the front of a pre that is after a (non-pre) paragraph.	The
text that was in the pre gets appended to the paragraph.  Any runs of
consecutive spaces anywhere in the old pre need to be converted to space-nbsp
combinations.

I think doing that might be beyond the scope of the editor (it is potentially a
performance problem).  So I've made limited fixes that are fast and that allow
normal editting behavior within pre's.

Comment 10

[kinmoz]

Comment on attachment 68847 [details] [diff] [review]
diffs for nsWSRunObject.h

sr=[e-mail]

Comment 11

[kinmoz]

Comment on attachment 68848 [details] [diff] [review]
diffs for nsWSRunObject.cpp

Are these intermediate assignments just for readability's sake, or can we
remove them?

+	    PRInt32 startOffset = point.mOffset;
+	    PRInt32 endOffset = point.mOffset+1;
+	    return DeleteChars(node, startOffset, node, endOffset);

The 2 blocks you added that call DeleteChars() need to have their indentation
fixed.

Heh, I was going to ask you why we didn't initialize outType, when looking at
one of your other patches:

+  *outType = eNone;

but then I realized "what's the point" if there's no code path that returns
without it being set?

sr=[e-mail]

Comment 12

[Joe Francis]

fixed on tip

Comment 13

[michael wendell]

From the testcases in bug 111974, it appears that this not totally fixed.
Tested on Win XP using 02-20 trunk build.

I am still seeing this issue if you:
1. Open Composer
2. Select Preformat from the paragraph drop-down
3. Type (without quotes) "    test"
4. Try to delete (either forward or backwards) the first t

Actual Results:
all but the first space is deleted.

Expected Results:
the t should be deleted and nothing else.

I am reopening this bug.

Comment 14

[Joe Francis]

This situation is a lot better now, but I think I can fix this issue too.

Comment 15

[Joe Francis]

pushed off EB+ get P1

Comment 16

[Joe Francis]

Attachment: patch to nsWSRunObject.cpp

additional tweak to fix the case mentioned in comment #13

Comment 17

[kinmoz]

Comment on attachment 74693 [details] [diff] [review]
patch to nsWSRunObject.cpp

sr=[e-mail]

Comment 18

[Kathleen :Brade]

Comment on attachment 74693 [details] [diff] [review]
patch to nsWSRunObject.cpp

r=brade

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 74693 [details] [diff] [review]
patch to nsWSRunObject.cpp

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 20

[Joe Francis]

fixed on trunk

Comment 21

[sujay]

Neil, can you verify this and mark verified-fixed? thanks

Comment 22

[[e-mail]]

This appears to have fixed all my five test cases.