Last Comment Bug 650505 - Get rid of ComputedCSSStyleDeclaration
: Get rid of ComputedCSSStyleDeclaration
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla6
Assigned To: Masatoshi Kimura [:emk]
:
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 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 Masatoshi Kimura [:emk] 2011-04-16 09:21:05 PDT
Created attachment 526500 [details] [diff] [review]
patch
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-16 10:25:47 PDT
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-7 (review requests must explain patch) 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 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).
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, jst@mozilla.com) 2011-04-18 15:12:04 PDT
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] 2011-05-02 03:21:18 PDT
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] 2011-05-03 06:00:26 PDT
Created attachment 529690 [details] [diff] [review]
patch for check in
Comment 8 Dão Gottwald [:dao] 2011-05-04 01:32:22 PDT
http://hg.mozilla.org/mozilla-central/rev/154900bdac90
Comment 9 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 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.