Last Comment Bug 378551 - origo sample table layout not working
: origo sample table layout not working
Status: RESOLVED FIXED
: fixed1.8.0.12, fixed1.8.1.4
Product: Core
Classification: Components
Component: XForms (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: aaronr
: Stephen Pride
Mentors:
http://www.mozilla.org/projects/xform...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-24 00:15 PDT by aaronr
Modified: 2007-04-24 19:17 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.40 KB, patch)
2007-04-24 00:32 PDT, aaronr
no flags Details | Diff | Review
testcase (3.63 KB, application/xhtml+xml)
2007-04-24 00:35 PDT, aaronr
no flags Details
patch (2.80 KB, patch)
2007-04-24 03:12 PDT, alexander :surkov
no flags Details | Diff | Review
patch (1.47 KB, patch)
2007-04-24 03:46 PDT, alexander :surkov
aaronr: review+
doronr: review+
Details | Diff | Review

Description aaronr 2007-04-24 00:15:00 PDT
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 2007-04-24 00:32:57 PDT
Created attachment 262613 [details] [diff] [review]
patch
Comment 2 aaronr 2007-04-24 00:35:11 PDT
Created attachment 262614 [details]
testcase
Comment 3 alexander :surkov 2007-04-24 03:12:28 PDT
Created attachment 262625 [details] [diff] [review]
patch

another approach, though your one looks more correct.
Comment 4 alexander :surkov 2007-04-24 03:46:17 PDT
Created attachment 262628 [details] [diff] [review]
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 2007-04-24 03:54:58 PDT
(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 2007-04-24 08:23:12 PDT
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) 2007-04-24 09:00:48 PDT
(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 2007-04-24 10:21:09 PDT
(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 2007-04-24 12:39:56 PDT
(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 2007-04-24 17:10:58 PDT
checked into trunk for surkov
Comment 11 aaronr 2007-04-24 19:17:23 PDT
checked into 1.8 and 1.8.0 branches

Note You need to log in before you can comment on or make changes to this bug.