39892 - Supporting the "behavior" propety with getComputedStyle()

Status: VERIFIED FIXED

(Core :: DOM: CSS Object Model, defect, P3)

Description

[Johnny Stenback (:jst)]

getComputedStyle() is currently implemented in mozilla but so far only a very
small set of properties are computed and properly supported, this is a tracking
bug intended to track the implementation of the various CSS properties that can
be gequested from the computed style.

The currently implemented properties are:

  left, top, right, bottom, width, hieght and display.

The DOM Level 2 CSS spec defines about 125 different properties but most of them
should be trivial to implement, and some are not supposed to be supported, like
"margin", in stead "margin-left", "margin-top", ... should be used.

The DOM Level 2 CSS spec is at

  http://www.w3.org/TR/DOM-Level-2/css.html

And the CSS2 spec is at

  http://www.w3.org/TR/REC-CSS2/

If you have spcial interest in some spcific property or a spcific set of
properties please file them as separate bugs and mark that this bug depends on
the new bug you file.

(the bug for getting getComputedStyle() into mozilla was bug 32169)

Comment 1

[David Hyatt]

*** Bug 42182 has been marked as a duplicate of this bug. ***

Comment 2

[David Hyatt]

Nominating for nsbeta2, since 42181 is now a problem.

Remember, the impl for this cannot rely on having a frame.  It can go to the 
frame for speed purposes, but if no frame exists, I still need the 
correct answer.

Comment 3

[Johnny Stenback (:jst)]

Attachment: getComputedStyle(...).getPropertyValue('behavior') impl.

Comment 4

[Johnny Stenback (:jst)]

The patch I just attached should fix this bug and also implement
getPropertyValue() which makes getComputedStyle() a whole lot easier to use.

Pierre, could you have a look at the patch and check that I'm not misusing the
stylesystem in some way, I do a ReResolveStyleContext() in there, that's OK,
right?

Pierre, when writing the patch I noticed that nsStyleUserInterface has a
nsString member but no destructor so I'm suspecting we'd leak the string, could
you have a peak at that?

Comment 5

[Johnny Stenback (:jst)]

Eh, the patch should not fix this bug, only parts of it (adds support for the
"behavior" property and implements getPropertyValue and also .cssText in
CSSPrimitiveValue).

David, there's simply *no* way we'd get *all* CSS2 properties supported in
getComputedStyle() for beta2, and that's what this bug is about. Preferably
there should be a separate bug on adding support for the behavior property, and
that could be fixed for beta2 (that's all you need for beta2, right?)...

Comment 6

[David Hyatt]

Yes, I just need behavior, and it has to work for display: none elements.

Comment 7

[David Hyatt]

Patch looks good.

Comment 8

[leger]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 9

[Pierre Saslawsky]

I'm not competent to do a real review of the patch, I can only say that it looks 
good. Don't we need to implement the code for more CSS_xxx types in 
nsROCSSPrimitiveValue::GetCssText()? I'm especially thinking of CSS_PC (picas) 
and CSS_NUMBER (for lengths of 0, meaning 0pt).

Thanks for pointing me to the memory leak in nsStyleUserInterface. I opened bug 
42326.

Comment 10

[Johnny Stenback (:jst)]

Changing summary to reflect that this is now about the behavior property, and
no other properties. I opened a new bug (bug 42417) for tracking all other CSS2
properties...

Pierre, you're right, nsROCSSPrimitiveValue::GetCssText() is incomplete but it
does even more than all it needs to do at this stage, eventually we'll need to
add support for more properties n' such...

Comment 11

[Johnny Stenback (:jst)]

I just checked in the fix for this, marking FIXED.

Comment 12

[leger]

Adding verifyme keyword.

Comment 13

[vidur (gone)]

Verified fixed.