216838 - maxHeight property of XUL element's computed style does not reflect the actual value (need style mapping of attributes)

Status: NEW

(Core :: XUL, defect)

Description

[Wang Xianzhu]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030820
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030820

See steps to reproduce

Reproducible: Always

Steps to Reproduce:
1. open the attachment document (test1.xul)
2. click on the red bar
3. read the alert messages

Actual Results:  
When the script setting ele.height to 100 and ele.maxHeight to 50, the computed
style's height property changes to 50px and maxHeight remains original 40px. 

Expected Results:  
The effective max-height style after setting ele's maxHeight property should be
50px, and it indeed 'effective', because the actual height is limited to 50px. 
The maxHeight property of computed style should reflect this 'effective' value: 50px

why is XULElement's 'style' property unimplemented?

Comment 1

[Wang Xianzhu]

Attachment: test1.xul

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is a XUL issue.  The XUL layout code uses the maxheight attribute directly
instead of mapping it into CSS, hence the style system doesn't know anything
about the presence of the maxheight attribute.

David, do you think we should introduce attribute mapping to XUL?  Is it
possible to get in touch with hyatt (he's ignored my last few mails, as far as I
can tell...)?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think it might speed up dynamic attribute changes in XUL a good bit, and is
likely to make the code cleaner, although not necessarily smaller, and would
significantly improve support for the corresponding CSS properties.  I certainly
wouldn't mind, but it's probably a good bit of work.  I might be forgetting
something, though.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attribute mapping would have to resolve the cascade ordering issue -- where
should XUL attributes come in the cascade?

Comment 5

[David Hyatt]

Right now XUL attributes beat author rules, which is different from how HTML attributes work.  I 
suspect that attribute mapping could work fine (and still be properly backwards compatible) if you gave 
mapped attributes an infinite specificity in the author cascade (right now XUL attributes even beat inline 
style rules).

Comment 6

[Boris Zbarsky [:bzbarsky]]

I'm actually thinking of making the cascade look like this (from my
n.p.m.style/xpfe post):

From most important to least important:

ua !important
user !important
author !important
attributes
inline style
author
user
ua 

for XUL.  What do you think, hyatt?  Would it be OK to have !important author
rules beating attributes?

Comment 7

[David Hyatt]

I think that would be ok.

Comment 8

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 9

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 10

[Boris Zbarsky [:bzbarsky]]

So the following attributes can be mapped, I think:

  width, height, maxwidth, maxheight, minwidth, minheight
  align (the non-deprecated version), pack
  orient
  flex
  left, top (pixel values only?)
  ordinal

And maybe mapped:

  dir (mapped into two different structs... how does CSS/attr interaction work?)
  

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note: There's code in nsIBox::AddCSSPrefSize that can go away if we do this.

Also note:  dbaron thinks that multiple XBL bindings in same doc should share the mapping style rules (so we shouldn't, e.g. make the rules per-element things).

Comment 12

[Eli Friedman]

Attachment: WIP

I put this together because I was wondering how difficult it would be... this patch is kind of working, although buggy in some way I haven't completely figured out.

I'm not sure the approach is actually sound, because I don't think it can be extended easily to work with lightweight elements.  I might as well throw it up anyway.

This doesn't include any layout/ changes... that stuff essentially will just end up being cleanup anyway.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Style set changes

You'll need this too.  And you'll need to put the attr rules in the right level...