Closed
Bug 780146
Opened 13 years ago
Closed 13 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•13 years ago
|
||
Attachment #648693 -
Flags: review?(jst)
| Assignee | ||
Comment 2•13 years ago
|
||
Attachment #648694 -
Flags: review?(jst)
| Assignee | ||
Comment 3•13 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•13 years ago
|
||
I don't see the point of having this function.
Attachment #648697 -
Flags: review?(peterv)
Comment 5•13 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•13 years ago
|
||
Comment on attachment 648693 [details] [diff] [review]
Part a: _content
Oh, but you should bump the iid for nsIDOMJSWindow.
Updated•13 years ago
|
Attachment #648694 -
Flags: review?(jst) → review+
Updated•13 years ago
|
Attachment #648696 -
Flags: review?(jst) → review+
| Assignee | ||
Comment 7•13 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•13 years ago
|
||
Updated•13 years ago
|
Attachment #649663 -
Flags: review?(jst) → review+
| Assignee | ||
Comment 9•13 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•13 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•13 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•13 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•13 years ago
|
||
Just guessing. Could bug 781525 a side effect of these patches landing ?
Mike Hommey """thinks""" so.
Comment 14•13 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•13 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•13 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•13 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•13 years ago
|
||
WebIDL does not allow identifiers beginning with underscore either.
http://dev.w3.org/2006/webapi/WebIDL/#proddef-ArgumentNameKeyword
Comment 19•13 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•13 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•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
| Assignee | ||
Comment 21•13 years ago
|
||
Flags: in-testsuite-
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•