Closed
Bug 1394694
Opened 7 years ago
Closed 7 years ago
Remove nsIAtom usage from some XUL-related IDL files
Categories
(Core :: XUL, enhancement)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(2 files, 3 obsolete files)
2.05 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
8.53 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
Bug 1392883 is about deCOMtaminating nsIAtom. I'm currently leaning toward a full deCOMtamination, i.e. removing nsIAtom.idl altogether. Which means that no other .idl file can mention nsIAtom outside of `%{C++ ... %}` blocks. So this bug is about removing all such nsIAtom mentions from some XUL-related .idl files.
Assignee | ||
Comment 1•7 years ago
|
||
It's unused; the zero-arg nsITreeColumn::GetAtom() is used everywhere instead.
Attachment #8902116 -
Flags: review?(jvarga)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
This requires converting two methods to C++-only functions.
Attachment #8902119 -
Flags: review?(jvarga)
Assignee | ||
Comment 3•7 years ago
|
||
This requires converting one method to a C++-only function.
Attachment #8902121 -
Flags: review?(jvarga)
Assignee | ||
Comment 4•7 years ago
|
||
This requires converting three methods to C++-only functions.
Attachment #8902122 -
Flags: review?(jvarga)
Comment 5•7 years ago
|
||
Comment on attachment 8902119 [details] [diff] [review] (part 2) - Remove nsIAtom use from nsIXULTemplateResult Review of attachment 8902119 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/xul/templates/nsIXULTemplateResult.idl @@ +8,2 @@ > interface nsIDOMNode; > interface nsIRDFResource; What about adding this here: %{ C++ class nsIAtom; %} [ptr] native nsIAtomPtr(nsIAtom); @@ +79,5 @@ > * @param aVar the variable to look up > * > * @return the value for the variable or a null string if it has no value > */ > + virtual nsresult GetBindingFor(nsIAtom* aVar, nsAString& aValue) = 0; and then: [noscript] AString getBindingFor(in nsIAtomPtr aVar); This way, you don't need to update all the .h/cpp files I think
Assignee | ||
Updated•7 years ago
|
Attachment #8902119 -
Attachment is obsolete: true
Attachment #8902119 -
Flags: review?(jvarga)
Assignee | ||
Updated•7 years ago
|
Attachment #8902121 -
Attachment is obsolete: true
Attachment #8902121 -
Flags: review?(jvarga)
Assignee | ||
Updated•7 years ago
|
Attachment #8902122 -
Attachment is obsolete: true
Attachment #8902122 -
Flags: review?(jvarga)
Assignee | ||
Comment 6•7 years ago
|
||
> What about adding this here:
> %{ C++
> class nsIAtom;
> %}
Thank you for the suggestion. Turns out we can keep the |interface nsIAtom;| forward declaration. That's fine so long as nsIAtom is only used in [noscript] methods.
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8904371 -
Flags: review?(jvarga)
Assignee | ||
Comment 8•7 years ago
|
||
janv: I replaced the old parts 2, 3, and 4 with a new part 2 that is simpler. Part 1 is still reasonable, though.
Comment 9•7 years ago
|
||
Comment on attachment 8902116 [details] [diff] [review] (part 1) - Remove nsITreeColumn.atom Review of attachment 8902116 [details] [diff] [review]: ----------------------------------------------------------------- I assume all relevant implementations of nsITreeView don't use the atom anymore.
Attachment #8902116 -
Flags: review?(jvarga) → review+
Comment 10•7 years ago
|
||
Comment on attachment 8904371 [details] [diff] [review] (part 2) - Mark nsIAtom-using methods in nsIXUL*.idl as [noscript] Review of attachment 8904371 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks!
Attachment #8904371 -
Flags: review?(jvarga) → review+
Assignee | ||
Comment 11•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b191c933df2076747864be07b8e7fc402a54a687 Bug 1394694 (part 1) - Remove nsITreeColumn.atom. r=janv. https://hg.mozilla.org/integration/mozilla-inbound/rev/33e34e8290927b41a59cf66ab9fac3150283236e Bug 1394694 (part 2) - Mark nsIAtom-using methods in nsIXUL*.idl as [noscript]. r=janv.
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b191c933df20 https://hg.mozilla.org/mozilla-central/rev/33e34e829092
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 13•6 years ago
|
||
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in
before you can comment on or make changes to this bug.
Description
•