Closed Bug 780146 Opened 13 years ago Closed 13 years ago

Various DOMClassInfo cleanup

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

Details

Attachments

(6 files)

I've got a few patches to simplify nsDOMClassInfo a bit.
Attached patch Part a: _contentSplinter Review
Attachment #648693 - Flags: review?(jst)
There are about four score and seven ways to define a constructor, so I think this one, at least, can go.
Attachment #648696 - Flags: review?(jst)
I don't see the point of having this function.
Attachment #648697 - Flags: review?(peterv)
Comment on attachment 648693 [details] [diff] [review] Part a: _content The reason we did this in the first place was for performance back in the day. I doubt it matters any more, and I'm guessing we don't use _content much any more.
Attachment #648693 - Flags: review?(jst) → review+
Comment on attachment 648693 [details] [diff] [review] Part a: _content Oh, but you should bump the iid for nsIDOMJSWindow.
Attachment #648694 - Flags: review?(jst) → review+
Attachment #648696 - Flags: review?(jst) → review+
This code can only be reached if ObjectIsNativeWrapper was false; I think it's clearer to move it into the if.
Attachment #649663 - Flags: review?(jst)
Attached patch diff -w (part e)Splinter Review
Attachment #649663 - Flags: review?(jst) → review+
Comment on attachment 648694 [details] [diff] [review] Part b: Inline nsDOMClassInfo::RegisterClassName https://hg.mozilla.org/mozilla-central/rev/b20779389b5a
Attachment #648694 - Flags: checkin+
Comment on attachment 648696 [details] [diff] [review] Part c: Don't define constructors by contract ID https://hg.mozilla.org/mozilla-central/rev/0e6e404c6baf
Attachment #648696 - Flags: checkin+
Comment on attachment 649663 [details] [diff] [review] Part e: Move all standard classes resolution code into ObjectIsNativeWrapper https://hg.mozilla.org/mozilla-central/rev/0f96a7f073a8
Attachment #649663 - Flags: checkin+
Just guessing. Could bug 781525 a side effect of these patches landing ? Mike Hommey """thinks""" so.
(In reply to :Ms2ger from comment #9) > Comment on attachment 648693 [details] [diff] [review] > Part a: _content > > https://hg.mozilla.org/mozilla-central/rev/c77acb256aec window._content has disappeared because of this change. A preceding underscore will be removed to make reserved words available as identifiers such as "char" attribute of KeyEvent (see bug 762758).
(In reply to Masatoshi Kimura [:emk] from comment #14) > (In reply to :Ms2ger from comment #9) > > Comment on attachment 648693 [details] [diff] [review] > > Part a: _content > > > > https://hg.mozilla.org/mozilla-central/rev/c77acb256aec > > window._content has disappeared because of this change. > A preceding underscore will be removed to make reserved words available as > identifiers such as "char" attribute of KeyEvent (see bug 762758). So, how am I supposed to declare an attribute whose name starts with an underscore?
(In reply to :Ms2ger from comment #16) > So, how am I supposed to declare an attribute whose name starts with an > underscore? It's impossible, AFAICT :( Identifiers can't start with an underscore even before bug 762758.
WebIDL does not allow identifiers beginning with underscore either. http://dev.w3.org/2006/webapi/WebIDL/#proddef-ArgumentNameKeyword
Comment on attachment 648697 [details] [diff] [review] Part d: Inline mozilla::dom::binding::DefineConstructor r=me
Attachment #648697 - Flags: review?(peterv) → review+
Comment on attachment 648697 [details] [diff] [review] Part d: Inline mozilla::dom::binding::DefineConstructor Landed for Gecko 18: https://hg.mozilla.org/mozilla-central/rev/bbf6b499528a
Attachment #648697 - Flags: checkin+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 798014
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: