CSS class change result in inconsistent layout

[Adam Wu]

As shown in the sample html:

The element <label> initially has a class name "Top", which is associated to style "display: block"; this makes the label displayed above the text input;

Then, by clicking on the "Right" button, the class name of the label is reassigned to "Right", which is associated to style "float: right". The label now should be displayed on the same line with the text input, to the right; but on Safari 3.2.1, the label is still displayed above the text input, and become right aligned.

However, again on Safari 3.2.1, if the "Left" button (which has no style associated to it) is clicked first (essentially removes style "display: block"), then the "Right" button, the label's layout is rendered as expected.

[Adam Wu]

Created attachment 26628 [details]
Sample html

[Adam Wu]

Google Chrome (1.0.154) also has the problem.

[Alexey Proskuryakov]

Confirmed with r41058; this is not a regression from Safari/WebKit 3.2.1.

[Dave Hyatt]

This bug is tricky.

The layout issue is basically caused by our inability to compute the correct m_maxPrefWidth for a rendering like this:

<div style="position:absolute">
<div style="float:right">Hello</div>
<div><input type=text></div>
</div>

It's hard for us to tell whether the float is going to fit into the next block in calcBlockPrefWidths.

In the real test case attached to the bug, the initial display:block on the first div causes us to create an anonymous block to wrap the input field.  When the first div is switched to just being floating instead, we could technically revert back to being inline again and remove the anonymous block (pulling its kids back up).

So either trying to fix maxPrefWidth or trying to make anonymous blocks go away properly when float/position changes would fix the bug.

[Neelima]

I have done some analysis on this bug. 
1)I found issue in Node::Diff()
I have done some changes in Node::Diff () by adding one more condition, which will detach the previous node with their respective properties and again attach with new properties to the node.
Node::StyleChange Node::diff( RenderStyle *s1, RenderStyle *s2 )
{
.........
.........
else  if (*s1 != *s2)
    ch = Detach; 
	OR
else  if (display1 == display2 || fl1 == fl2 ||
 (s1 && s2 && s1->contentDataEquivalent(s2)))
	ch = Detach;
.........
.........
}
	I have tested and the fix seems to be working fine. Kindly let me know whether this is a correct approach?

2)Need Clarification: I tried with another approach to resolve the same issue.
Whenever  select  from left to right or Right to Top, the style is setting to Block level in CSSStyleSelectos::adjustRenderStylefunction() of  CSSStyleSelector.cpp,  I had  tried to change ‘setDisplay’ value to inline for the ‘case :Top to Right’ but it’s not working. I found that in CSSStyleSelectos::adjustRenderStylefunction() , they are forcefully setting inline-level roots to  be block-level , if someone attempts to float an inline. See the below code:
void CSSStyleSelector::adjustRenderStyle(RenderStyle* style, Element *e)
{
// Mutate the display to BLOCK or TABLE for certain cases, e.g., if //someone attempts to position or float an inline, compact, or run-in.  //Cache the original display, since it may be needed for positioned //elements that have to compute their static normal flow positions.  //We also force inline-level roots to be block-level.
if (style->display() != BLOCK && style->display() != TABLE && style->display() != BOX && (style->position() == AbsolutePosition || style->position() == FixedPosition || style->floating() != FNONE ||
             (e && e->document()->documentElement() == e))) {
            if (style->display() == INLINE_TABLE)
                style->setDisplay(TABLE);
            else if (style->display() == INLINE_BOX)
                style->setDisplay(BOX);
            else if (style->display() == LIST_ITEM) {
  // It is a WinIE bug that floated list items lose their //bullets, so we'll emulate the quirk, but only in quirks mode.
                if (!m_checker.m_strictParsing && style->floating() != FNONE)
                    style->setDisplay(BLOCK);
            }
else
         style->setDisplay(BLOCK); // This part of code is getting hit when I					// change from left to right or right to top 
	

        }
Please let me know why it has been done like this?

[john stalcup]

this bug appears to be fixed now

[Robert Hogan]

(In reply to comment #6)
> this bug appears to be fixed now

No, it's still valid.

[Brent Fulgham]

Safari, Chrome, and Firefox show the same rendering behavior for this test case. I do not believe any further compatibility issue remains.