Closed
Bug 780146
Opened 11 years ago
Closed 11 years ago
Various DOMClassInfo cleanup
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(6 files)
6.75 KB,
patch
|
jst
:
review+
Ms2ger
:
checkin-
|
Details | Diff | Splinter Review |
2.53 KB,
patch
|
jst
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
7.68 KB,
patch
|
jst
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
2.63 KB,
patch
|
bzbarsky
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
4.36 KB,
patch
|
jst
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
2.43 KB,
patch
|
Details | Diff | Splinter Review |
I've got a few patches to simplify nsDOMClassInfo a bit.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #648693 -
Flags: review?(jst)
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #648694 -
Flags: review?(jst)
Assignee | ||
Comment 3•11 years ago
|
||
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)
Assignee | ||
Comment 4•11 years ago
|
||
I don't see the point of having this function.
Attachment #648697 -
Flags: review?(peterv)
Comment 5•11 years ago
|
||
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 6•11 years ago
|
||
Comment on attachment 648693 [details] [diff] [review] Part a: _content Oh, but you should bump the iid for nsIDOMJSWindow.
Updated•11 years ago
|
Attachment #648694 -
Flags: review?(jst) → review+
Updated•11 years ago
|
Attachment #648696 -
Flags: review?(jst) → review+
Assignee | ||
Comment 7•11 years ago
|
||
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)
Assignee | ||
Comment 8•11 years ago
|
||
Updated•11 years ago
|
Attachment #649663 -
Flags: review?(jst) → review+
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 648693 [details] [diff] [review] Part a: _content https://hg.mozilla.org/mozilla-central/rev/c77acb256aec
Attachment #648693 -
Flags: checkin+
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 648694 [details] [diff] [review] Part b: Inline nsDOMClassInfo::RegisterClassName https://hg.mozilla.org/mozilla-central/rev/b20779389b5a
Attachment #648694 -
Flags: checkin+
Assignee | ||
Comment 11•11 years ago
|
||
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+
Assignee | ||
Comment 12•11 years ago
|
||
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+
Comment 13•11 years ago
|
||
Just guessing. Could bug 781525 a side effect of these patches landing ? Mike Hommey """thinks""" so.
Comment 14•11 years ago
|
||
(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).
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 648693 [details] [diff] [review] Part a: _content https://hg.mozilla.org/integration/mozilla-inbound/rev/eac51122b5aa
Attachment #648693 -
Flags: checkin+ → checkin-
Assignee | ||
Comment 16•11 years ago
|
||
(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?
Comment 17•11 years ago
|
||
(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.
Comment 18•11 years ago
|
||
WebIDL does not allow identifiers beginning with underscore either. http://dev.w3.org/2006/webapi/WebIDL/#proddef-ArgumentNameKeyword
![]() |
||
Comment 19•11 years ago
|
||
Comment on attachment 648697 [details] [diff] [review] Part d: Inline mozilla::dom::binding::DefineConstructor r=me
Attachment #648697 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 20•11 years ago
|
||
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+
Assignee | ||
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 21•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bbf6b499528a
Flags: in-testsuite-
Updated•4 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•