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)
5.49 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
2.52 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
1.97 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
4.63 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
7.18 KB,
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•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•