Closed Bug 1394694 Opened 7 years ago Closed 7 years ago

Remove nsIAtom usage from some XUL-related IDL files

Categories

(Core :: XUL, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(2 files, 3 obsolete files)

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.
It's unused; the zero-arg nsITreeColumn::GetAtom() is used everywhere instead.
Attachment #8902116 - Flags: review?(jvarga)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
This requires converting two methods to C++-only functions.
Attachment #8902119 - Flags: review?(jvarga)
This requires converting one method to a C++-only function.
Attachment #8902121 - Flags: review?(jvarga)
This requires converting three methods to C++-only functions.
Attachment #8902122 - Flags: review?(jvarga)
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
Attachment #8902119 - Attachment is obsolete: true
Attachment #8902119 - Flags: review?(jvarga)
Attachment #8902121 - Attachment is obsolete: true
Attachment #8902121 - Flags: review?(jvarga)
Attachment #8902122 - Attachment is obsolete: true
Attachment #8902122 - Flags: review?(jvarga)
> 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.
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 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 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+
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.