Closed Bug 378551 Opened 18 years ago Closed 18 years ago

origo sample table layout not working

Categories

(Core Graveyard :: XForms, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aaronr, Assigned: aaronr)

References

()

Details

(Keywords: fixed1.8.0.12, fixed1.8.1.4)

Attachments

(3 files, 1 obsolete file)

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.
Attached patch patchSplinter Review
Attached file testcase
Attached patch patch (obsolete) — Splinter Review
another approach, though your one looks more correct.
Attached patch patchSplinter Review
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.
Attachment #262625 - Attachment is obsolete: true
(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.
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.
(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"

(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?
(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.
Attachment #262628 - Flags: review+
Attachment #262628 - Flags: review?(doronr)
Attachment #262628 - Flags: review?(doronr) → review+
checked into trunk for surkov
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
checked into 1.8 and 1.8.0 branches
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: