Get rid of ComputedCSSStyleDeclaration

RESOLVED FIXED in mozilla6

Status

()

Core
DOM: CSS Object Model
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

unspecified
mozilla6
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 526500 [details] [diff] [review]
patch
Attachment #526500 - Flags: review?(dbaron)
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?
Attachment #526500 - Flags: review?(jst)
Attachment #526500 - Flags: review?(dbaron)
Attachment #526500 - Flags: review+
In particular, there's a DOMCI_DATA line that associates CSSStyleDeclaration with nsDOMCSSDeclaration, yet you're changing nsComputedDOMStyle to use CSSStyleDeclaration.
(Assignee)

Comment 4

6 years ago
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 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.
Attachment #526500 - Flags: review?(peterv)
Attachment #526500 - Flags: review?(jst)
Attachment #526500 - Flags: review+
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)
Attachment #526500 - Flags: review?(peterv) → review+
(Assignee)

Comment 7

6 years ago
Created attachment 529690 [details] [diff] [review]
patch for check in
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → VYV03354
http://hg.mozilla.org/mozilla-central/rev/154900bdac90
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla6

Comment 9

6 years ago
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?
(Assignee)

Comment 10

6 years ago
Use CSSStyleDeclaration instead.
You need to log in before you can comment on or make changes to this bug.