Last Comment Bug 650505 - Get rid of ComputedCSSStyleDeclaration
: Get rid of ComputedCSSStyleDeclaration
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla6
Assigned To: Masatoshi Kimura [:emk]
: Jet Villegas (:jet)
Depends on:
  Show dependency treegraph
Reported: 2011-04-16 09:18 PDT by Masatoshi Kimura [:emk]
Modified: 2011-09-02 08:31 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (3.84 KB, patch)
2011-04-16 09:21 PDT, Masatoshi Kimura [:emk]
dbaron: review+
jst: review+
peterv: review+
Details | Diff | Splinter Review
patch for check in (3.92 KB, patch)
2011-05-03 06:00 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Splinter Review

Description User image Masatoshi Kimura [:emk] 2011-04-16 09:18:21 PDT
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 User image Masatoshi Kimura [:emk] 2011-04-16 09:21:05 PDT
Created attachment 526500 [details] [diff] [review]
Comment 2 User image David Baron :dbaron: ⌚️UTC-8 2011-04-16 10:25:47 PDT
Comment on attachment 526500 [details] [diff] [review]

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 User image David Baron :dbaron: ⌚️UTC-8 2011-04-16 10:27:14 PDT
In particular, there's a DOMCI_DATA line that associates CSSStyleDeclaration with nsDOMCSSDeclaration, yet you're changing nsComputedDOMStyle to use CSSStyleDeclaration.
Comment 4 User image Masatoshi Kimura [:emk] 2011-04-16 11:02:49 PDT
FYI, I found another example one classinfo (HTMLCollection) is shared by two classes (nsFormControlList and TableRowsCollection).
Comment 5 User image Johnny Stenback (:jst, 2011-04-18 15:12:04 PDT
Comment on attachment 526500 [details] [diff] [review]

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 User image Peter Van der Beken [:peterv] 2011-05-02 03:21:18 PDT
Comment on attachment 526500 [details] [diff] [review]

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

>   NS_INTERFACE_TABLE3(nsComputedDOMStyle,
>                       nsICSSDeclaration,
>                       nsIDOMCSSStyleDeclaration,
>                       nsIDOMCSS2Properties)

I think this should just be:

Comment 7 User image Masatoshi Kimura [:emk] 2011-05-03 06:00:26 PDT
Created attachment 529690 [details] [diff] [review]
patch for check in
Comment 8 User image Dão Gottwald [:dao] 2011-05-04 01:32:22 PDT
Comment 9 User image Biel Frontera 2011-09-02 03:02:14 PDT
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 User image Masatoshi Kimura [:emk] 2011-09-02 08:31:22 PDT
Use CSSStyleDeclaration instead.

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