Closed
Bug 1277401
Opened 8 years ago
Closed 8 years ago
Support [Func=""] on dictionary members
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: xidorn, Assigned: bzbarsky)
References
Details
(Whiteboard: btpp-active)
Attachments
(5 files)
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
It would be useful to support controling availability of a dictionary member based on a function (rather than just ChromeOnly), so that we can limit it's usage with functions, e.g. IsChromeOrXBL.
We would want to use this for EventListenerOptions.mozSystemGroup. See bug 1274520 comment 52.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8759234 -
Flags: review?(peterv)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8759235 -
Flags: review?(peterv)
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8759236 -
Flags: review?(peterv)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8759237 -
Flags: review?(peterv)
Assignee | ||
Comment 5•8 years ago
|
||
The example codegen from this patch looks like this. In
DictWithConditionalMembers::Init:
if (!isNull) {
if (nsContentUtils::ThreadsafeIsCallerChrome() &&
nsGenericHTMLElement::TouchEventsEnabled(cx, *object)) {
if (!JS_GetPropertyById(cx, *object, atomsCache->chromeOnlyFuncControlledMember_id, temp.ptr())) {
return false;
}
} else {
temp->setUndefined();
}
}
...
if (!isNull) {
if (nsContentUtils::ThreadsafeIsCallerChrome()) {
if (!JS_GetPropertyById(cx, *object, atomsCache->chromeOnlyMember_id, temp.ptr())) {
return false;
}
} else {
temp->setUndefined();
}
}
...
if (!isNull) {
if (TestFuncControlledMember(cx, *object)) {
if (!JS_GetPropertyById(cx, *object, atomsCache->funcControlledMember_id, temp.ptr())) {
return false;
}
} else {
temp->setUndefined();
}
}
For comparison, an unconditional member now looks like this:
if (!isNull) {
if (!JS_GetPropertyById(cx, *object, atomsCache->a_id, temp.ptr())) {
return false;
}
}
And when converting the dictionary to a JS object, we now have:
{
if (nsContentUtils::ThreadsafeIsCallerChrome() &&
nsGenericHTMLElement::TouchEventsEnabled(cx, obj)) {
if (mChromeOnlyFuncControlledMember.WasPassed()) {
do {
// the actual conversion here
}
}
}
if (nsContentUtils::ThreadsafeIsCallerChrome()) {
if (mChromeOnlyMember.WasPassed()) {
do {
// the actual conversion here
}
}
}
if (TestFuncControlledMember(cx, obj)) {
if (mFuncControlledMember.WasPassed()) {
do {
// the actual conversion here
}
}
}
Attachment #8759248 -
Flags: review?(peterv)
Updated•8 years ago
|
Whiteboard: btpp-active
Comment 6•8 years ago
|
||
Comment on attachment 8759234 [details] [diff] [review]
part 1. Factor out the condition list generation for pref/func so we can reuse it for dictionary members
Review of attachment 8759234 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/bindings/Codegen.py
@@ +3339,5 @@
> + if func:
> + assert isinstance(func, list) and len(func) == 1
> + conditions.append("%s(aCx, %s)" % (func[0], objName))
> + if idlobj.getExtendedAttribute("SecureContext"):
> + conditions.append("mozilla::dom::IsSecureContextOrObjectIsFromSecureContext(aCx, aObj)")
Shouldn't this use objName instead of aObj?
Attachment #8759234 -
Flags: review?(peterv) → review+
Updated•8 years ago
|
Attachment #8759235 -
Flags: review?(peterv) → review+
Updated•8 years ago
|
Attachment #8759236 -
Flags: review?(peterv) → review+
Updated•8 years ago
|
Attachment #8759237 -
Flags: review?(peterv) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8759248 [details] [diff] [review]
part 5. Change dictionary codegen to output Func stuff as needed
Review of attachment 8759248 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/bindings/Codegen.py
@@ +3348,2 @@
> if idlobj.getExtendedAttribute("SecureContext"):
> + conditions.append("mozilla::dom::IsSecureContextOrObjectIsFromSecureContext(%s, %s)" % (cxName, objName))
Ah, here it's changed to objName, ok.
Attachment #8759248 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 8•8 years ago
|
||
> Ah, here it's changed to objName, ok.
But you're right: it should move to part 1. Will do that.
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bca03ed2ecb
part 1. Factor out the condition list generation for pref/func so we can reuse it for dictionary members. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/42aa63e1c573
part 2. Fix up includes for dictionaries that have [ChromeOnly] members. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd91042c6a8b
part 3. Add IDL parser support for [Func] on dictionary members. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/759fdef36b82
part 4. Fix up includes for dictionary members with [Func] on them. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/44182df66982
part 5. Change dictionary codegen to output Func stuff as needed. r=peterv
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8bca03ed2ecb
https://hg.mozilla.org/mozilla-central/rev/42aa63e1c573
https://hg.mozilla.org/mozilla-central/rev/cd91042c6a8b
https://hg.mozilla.org/mozilla-central/rev/759fdef36b82
https://hg.mozilla.org/mozilla-central/rev/44182df66982
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
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
•