378551 - origo sample table layout not working

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

Looks like the table layout styling in the Origo xforms sample (http://www.mozilla.org/projects/xforms/samples/insurance_form/app_certificates.xhtml) is no longer working.

I tracked this down to a change made in the patch for bug 339286.  We are now setting the style.display property of the explicit or implicit label anonymous content when we want it to become visible.  This will set the @style="display:...;" style of the anonymous content.  This will override whatever CSS style we are trying to apply to it, like the "display: table-cell; width: 90px;" style that the Origo form is trying to set.

I've got this working again by setting style.display="" on the anonymous content to make it visible.

Comment 1

[aaronr]

Attachment: patch

Comment 2

[aaronr]

Attachment: testcase

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

another approach, though your one looks more correct.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

it's the same patch like yours one (see http://lxr.mozilla.org/mozilla/source/layout/style/nsDOMCSSDeclaration.cpp#210).

When property is removed or changed on empty value then it looks the property is recalculated therefore yours approach is right.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)

> When property is removed or changed on empty value then it looks the property
> is recalculated therefore yours approach is right.
> 

Though it looks it's more correct to remove it :) (see http://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSStyleDeclaration)

removeProperty
    Used to remove a CSS property if it has been explicitly set within this declaration block.

setProperty
    Used to set a property value and priority within this declaration block.

Though I don't know good the specs and the code probably I misunderstand something.

Comment 6

[aaronr]

I've never used the removeProperty stuff before, so I can't say much about it.  Doron knows CSS pretty well, I'll ask him if removeProperty has any drawbacks or if we should use it in this case.

Comment 7

[Doron Rosenberg (IBM)]

(In reply to comment #6)
> I've never used the removeProperty stuff before, so I can't say much about it. 
> Doron knows CSS pretty well, I'll ask him if removeProperty has any drawbacks
> or if we should use it in this case.
> 

Doing style.display="" is wrong, since "" is an invalid value.  I would either use removeProperty or do .style.display = "inherit"

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #7)
> (In reply to comment #6)
> > I've never used the removeProperty stuff before, so I can't say much about it. 
> > Doron knows CSS pretty well, I'll ask him if removeProperty has any drawbacks
> > or if we should use it in this case.
> > 
> 
> Doing style.display="" is wrong, since "" is an invalid value.  I would either
> use removeProperty or do .style.display = "inherit"
> 

Doron, if CSS style rules are applied to the element and f I set "inherit" value then will those rules be used or "inherit" means inherit style from parent element?

Comment 9

[aaronr]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > I've never used the removeProperty stuff before, so I can't say much about it. 
> > > Doron knows CSS pretty well, I'll ask him if removeProperty has any drawbacks
> > > or if we should use it in this case.
> > > 
> > 
> > Doing style.display="" is wrong, since "" is an invalid value.  I would either
> > use removeProperty or do .style.display = "inherit"
> > 
> 
> Doron, if CSS style rules are applied to the element and f I set "inherit"
> value then will those rules be used or "inherit" means inherit style from
> parent element?
> 

Doron says that removeProperty is better since using 'inherit' will get the style from the parent.  He says setting the value to an empty string will work, but it is mozilla only and not the 'correct' way to do it.  So we'll go with the latest Alex patch.

Comment 10

[aaronr]

checked into trunk for surkov

Comment 11

[aaronr]

checked into 1.8 and 1.8.0 branches