650505 - Get rid of ComputedCSSStyleDeclaration

Status: RESOLVED FIXED

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

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Open Web Console
2. Type getComputedStyle(document.body,"") into Web Console.

Actual result:
[object ComputedCSSStyleDeclaration] 

Expected result:
[object CSSStyleDeclaration] 

All other browsers work as expected. DOM Level 2 CSS doesn't define ComputedCSSStyleDeclaration. getComputedStyle should not expose implementation details to content.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: patch

Comment 2

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

Comment on attachment 526500 [details] [diff] [review]
patch

looks ok to me, but somebody more familiar with classinfo should review this -- in particular, is it ok for two different classes to have the same classinfo?

Comment 3

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

In particular, there's a DOMCI_DATA line that associates CSSStyleDeclaration with nsDOMCSSDeclaration, yet you're changing nsComputedDOMStyle to use CSSStyleDeclaration.

Comment 4

[Masatoshi Kimura [:emk]]

FYI, I found another example one classinfo (HTMLCollection) is shared by two classes (nsFormControlList and TableRowsCollection).
https://hg.mozilla.org/mozilla-central/annotate/b140e7746652/content/html/content/src/nsHTMLTableElement.cpp#l165
https://hg.mozilla.org/mozilla-central/annotate/b140e7746652/content/html/content/src/nsHTMLFormElement.cpp#l2288

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 526500 [details] [diff] [review]
patch

Looks good to me, but I'd like peterv to sign off on this as well as he's more familiar with all the perf work that's happened here lately.

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 526500 [details] [diff] [review]
patch

>diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp

> NS_INTERFACE_TABLE_HEAD(nsComputedDOMStyle)
>   NS_INTERFACE_TABLE3(nsComputedDOMStyle,
>                       nsICSSDeclaration,
>                       nsIDOMCSSStyleDeclaration,
>                       nsIDOMCSS2Properties)
>   NS_INTERFACE_TABLE_TO_MAP_SEGUE
>   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
>   NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsComputedDOMStyle)
>-  NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(ComputedCSSStyleDeclaration)
>+  NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(CSSStyleDeclaration)
> NS_INTERFACE_MAP_END

I think this should just be:

NS_INTERFACE_MAP_BEGIN(nsComputedDOMStyle)
  NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
  NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsComputedDOMStyle)
NS_INTERFACE_MAP_END_INHERITING(nsDOMCSSDeclaration)

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: patch for check in

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/154900bdac90

Comment 9

[Biel Frontera]

I've found a problem in an old webapp that uses ComputedCSSStyleDeclaration, and I think it can be related to this bug.

When I try to do var cs=ComputedCSSStyleDeclaration.prototype;
it throws this error: ComputedCSSStyleDeclaration is not defined

This works perfecty in Firefox4, but fails in firefox6.

Do you know how to avoid this problem? Do I have to create a new bug?

Comment 10

[Masatoshi Kimura [:emk]]

Use CSSStyleDeclaration instead.