108035 - Content out of order when making list of Divs and Table elements

Status: VERIFIED FIXED

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

Description

[kinmoz]

So if I have the following content in composer:


  This<div>Is</div><div>Some</div><div>List</div>Fun!


it appears on screen like this:


  This

  Is

  Some

  List

  Fun!


If I select all of the content and hit the list button, I get one list item that 
looks like this:


  * ThisFun!ListSomeIs


Some peculiar things about this:

  1. Why are we not treating divs as blocks and giving them their own
     list item? It looks like we are pulling out the div and table
     element contents and putting them directly in the list of nodes
     we are operating on in WillMakeList().

  2. It seems that GetListActionNodes() can actually cause the reordering
     of content in the array of nodes when they are divs or table elements,
     because it always appends them to the end of the array after they've
     stripped off the div/table element container.

Comment 1

[kinmoz]

Attachment: Test Case

Comment 2

[kinmoz]

Whoops I didn't mean to put empty lines between each of the words above in the 
section "it appears on screen like this:" it should look like this:

  This
  Is
  Some
  List
  Fun!

Comment 3

[Joe Francis]

kin: you want to work on this?

Comment 4

[kinmoz]

--> mozilla0.9.7

Comment 5

[kinmoz]

--> mozilla0.9.8

Comment 6

[kinmoz]

--> 0.9.9 (Load balancing)

Comment 7

[Syd Logan]

plussing

Comment 8

[Kevin McCluskey (gone)]

Reassigning to Daniel, since Kin is overloaded with EDITORBASE+ bugs.

Comment 9

[kinmoz]

Looks like the ordering problem is fixed .. we're still putting everything in a
single list item though.

Comment 10

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

accepting bug

Comment 11

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

strange... i never received the bugmails relative to this bug, including
reassignment.

Comment 12

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Ok I see. The root of the problem is in
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLEditRules.cpp#5286

We just cannot remove the nsHTMLEditUtils::IsDiv(testNode) test because of the
following example :

    <div>a first line of text
      <ul>
        <li>first item</li>
        <li>second item</li>
      </ul>
     </div>

select all and click on ordered list button. You want the whole thing to
become a single ordered list. If I remove the test above, the whole DIV
will be inserted into the first item of an ordered list.

This issue is trickier than we thought, I am afraid.

Comment 13

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

I think I found the cleanest way to solve the bug... If the last node of a div
we trying to put into a list is a text node, append a br after it. It should
solve all problems.

Comment 14

[Joe Francis]

why am i not surprised.  hmmm.  

One possibility is to change the loop that daniel indicates to append a null 
element to the list everytime we call GetInnerContent().  Then we could change 
WillMakeList(), etc, to interpret null elements in the list as a signal to create 
a new block {list item, etc} for further content.

Hackish, yes, but fairly simple.

Comment 15

[Syd Logan]

adt2 for the partial fix

Comment 16

[Joe Francis]

assigning to me at daniels request

Comment 17

[Joe Francis]

Attachment: patch to nsHTMLEditRules.cpp

This should fix it.

Comment 18

[kinmoz]

Comment on attachment 82424 [details] [diff] [review]
patch to nsHTMLEditRules.cpp

sr=[e-mail]

Comment 19

[Joe Francis]

fixed on trunk

Comment 20

[michael wendell]

Verified on the 05-14 trunk build.

Comment 21

[Kevin McCluskey (gone)]

adt1.0.0

Comment 22

[Jaime Rodriguez, Jr.]

sorry, its late, and i am tired. but, did i miss something here. who r= this bug
before it went into the trunk?

Removing adt1.0.0, for now, until there is a r=.

Comment 23

[Joe Francis]

I can't remember who r'd.  Daniel, can you r= this?

Comment 24

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

I confirm that I told r=glazman *before* this fix was checked in during an IRC
conversation with Joe.

Comment 25

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 82424 [details] [diff] [review]
patch to nsHTMLEditRules.cpp

r=glazman, as said a month ago during an IRC discussion with Joe

Comment 26

[Kevin McCluskey (gone)]

nsbeta1-. The fix is too risky for the 1.0 branch.