77558 - table gets screwed up after toggling between Normal & Source modes

Status: VERIFIED FIXED

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

Description

[sujay]

using 4/24 build of netscape

1) launch netscape
2) launch composer
3) insert default 2 x 2 table
4) click on the HTML Source tab at bottom
5) click on Show All Tags tab at bottom
6) click on Normal tab

notice the table gets enlarged and the caret
becomes big..

Comment 1

[Kathleen :Brade]

We are stuffing in an extra <br> in each empty cell.  :-/

Comment 2

[kinmoz]

There are actually 2 problems here:

The first one ... The big caret is the result of bug #47040, which doesn't 
preserve the caret position when switching between the modes.

I believe the big caret is acceptable/expected if the caret is placed before the 
table.

The 2nd one ... Like brade said above we are inserting a 2nd set of <BR>s in 
blank table cells, which I assume is the result of some rules post processing. 
The rules shouldn't be adding them to the table cells because they aren't empty.

Comment 3

[Joe Francis]

I bet this is a regression caused by my IsEmptyNode() changes.  There are times 
when I want to consider blocks with just a br empty.  This isn't one of them.  I 
should probably break it out into two different calls:  "IsEmpty" and 
"LooksEmpty".  :-)

Comment 4

[Joe Francis]

Attachment: diffs of nsHTMLEditor.cpp

Comment 5

[Joe Francis]

patch to editor/base/nsHTMLEditor.cpp attached.  need r/sr

patch explained: it used to be that callers to IsEmptyNode() would indicate 
whether or not a br node with the special moz attribute counted in determining if 
a node was empty.  So if we were trying to figure out if we needed to add a br 
node, we would call with that param false, but if we were trying to figure out if 
a node "looked" empty, we would call with it true.

For my fixes to 71362 I started moving IsEmptyNode away from dependency on moz-br 
nodes, and towards a more general approach of simply counting the number of brs 
(of any kind) inside the node.  But I didn't do a complete job, which led to this 
bug.  Now IsEMptyNode no longer cares about moz-brs at all, and we are back to 
happy land.  

We are almost to the point where we could stop using moz-brs, but the selection 
adjusting code is still cares about them, so I can't rip them out completely yet.

Comment 6

[Kathleen :Brade]

this seems good to me (tested the patch on Mac build from yesterday) r=brade

Comment 7

[sujay]

I can verify after this bug gets marked RESOLVED-FIXED...is the patch
in the tree?

did someone forget to mark this fixed? just checking...

Comment 8

[Joe Francis]

no one forgot.  I still need sr.

Comment 9

[kinmoz]

FYI, I gave Joe a verbal sr=kin last week. I believe Joe checked in his 05/01/01
13:44 patch already. Not sure if he's keeping this open on the caret issue or not.

Comment 10

[Kathleen :Brade]

caret issue is a different bug (caret not being placed correctly when going 
between tabs)

Comment 11

[Simon Fraser [no longer active]]

r/sr=sfraser

Comment 12

[Joe Francis]

ooops, forgot to mark fixed

Comment 13

[Keyser Sose]

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

Comment 14

[Kathleen :Brade]

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

Comment 15

[sujay]

verified in 5/30 build.