[Webkit-unassigned] [Bug 100453] contentEditable: Style of List Item Leaks Through After List End

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=100453


Vani Hegde <vani.hegde at samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vani.hegde at samsung.com




--- Comment #2 from Vani Hegde <vani.hegde at samsung.com>  2013-08-02 08:16:21 PST ---
The issue here is while breaking out of empty <li>, the style applied to <li> bleeds out of it.

In CompositeEditCommand::breakOutOfEmptyListItem(), while breaking out of an empty list element, we save the style present in it and apply it to the newBlock irrespective of whether it is <li> or <div> or <p>.
IMO, there is no need of saving the style of empty <li> and applying it to newBlock. 

I may be wrong in thinking that there is no need to do this at all, nevertheless below is the explanation for why I think so.

The empty <li> might have any of the below styles associated with it
    1. Style particularly applied to it (Ex: <li style=color:red>br</li>)
    2. Style applied to all <li>s in the document (Ex: li{color:red;})
    3. Style applied to document as a whole (Ex: body{color:red;})

New block created can be either <li> or <div>/<p>.

Let's consider the case where newBlock created while breaking out is either <div> or <p>
    If <li> has style as per 1 or 2, it would be wrong to apply this style to newBlock (And we are doing it currently)
    If <li> has style as per 3, it would have been already handled while attaching style to element's renderer and we need not do anything.

Now, let's consider the case where newBlock is <li>
    If it's style due to case 1, I think it would be wrong to apply it to new <li>. In case we want to apply the style for this particular case we can do something like below
    if (!style->isEmpty() && newBlock->hasTagName(olTag)) //Applying style of empty <li> to newBlock only if it is an <li> again

    If <li> has style as per 2 or 3, it would have been already handled while attaching style to element's renderer and we need not do anything


With this change I have executed all layout tests and the only regression I found was editing/deleting/delete-first-list-item.html
This was added as part of changelist http://trac.webkit.org/changeset/15563 and I feel this test case is wrong in itself as it is expecting the style specifically applied to empty <li> to bleed out while breaking out.

We will have to rebaseline this test.

Eager to know your thoughts on this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.