The default bug view has changed. See this FAQ.

Replace HTMLElement quickstubs with new binding methods

RESOLVED FIXED in mozilla20

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: peterv, Assigned: peterv)

Tracking

Trunk
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 688401 [details] [diff] [review]
Support castable types in unions v1

Currently compilation fails for castable interfaces in union types because we don't include the binding headers in UnionConversion.h but then then try to use the PrototypeTraits etc.
Attachment #688401 - Flags: review?(bzbarsky)
Comment on attachment 688401 [details] [diff] [review]
Support castable types in unions v1

r=me
Attachment #688401 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 688409 [details] [diff] [review]
Remove hardcoded interface list in workers v1

It's annoying to have to change the dom/worker code just because we added an interface that has a longer prototype chain than the current max.
Attachment #688409 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

4 years ago
Created attachment 688411 [details] [diff] [review]
Add a constructor to callbacks to allow conversion between callback types v1
Attachment #688411 - Flags: review?(bzbarsky)
(Assignee)

Comment 5

4 years ago
Created attachment 688413 [details] [diff] [review]
Replace HTMLElement quickstubs with new binding methods v1
Attachment #688413 - Flags: review?(bzbarsky)
Comment on attachment 688409 [details] [diff] [review]
Remove hardcoded interface list in workers v1

I like.  Well, not the necessity, but the solution.  ;)
Attachment #688409 - Flags: review?(bzbarsky) → review+
Comment on attachment 688411 [details] [diff] [review]
Add a constructor to callbacks to allow conversion between callback types v1

r=me
Attachment #688411 - Flags: review?(bzbarsky) → review+
Comment on attachment 688413 [details] [diff] [review]
Replace HTMLElement quickstubs with new binding methods v1

>+#define EVENT(name_, id_, type_, struct_) /* nothing; handled by the nsINode */

s/the nsINode/nsINode/

>+    nsCOMPtr<nsIScriptGlobalObject> sgo = do_QueryInterface(win);           \
>+    nsGlobalWindow* globalWin = nsGlobalWindow::FromSupports(sgo);          \

Why not just QI to nsISupports, since FromSupports _has_ to work with the canonical nsISupports?

Same in a few more places.

>+     return new EventHandlerNonNull(errorHandler);                         \

I would much rather we had this method return already_AddRefed<EventHandlerNonNull> and addref inside here as needed, to make sure that the caller is not just sticking it into a raw ptr.

As a side note, maybe we should allow resultNotAddRefed for callbacks?

>+  OnErrorEventHandlerNonNull* errorHandler =                                \
>+    new OnErrorEventHandlerNonNull(handler);                                \

nsRefPtr, please, so we're not passing in a 0-refcount object.

>+++ b/content/html/content/src/nsHTMLBodyElement.cpp

Could you file a followup on using some of the existing nsDOMEventTargetHelper bits in this code?

r=me with the above nits fixed.
Attachment #688413 - Flags: review?(bzbarsky) → review+
Comment on attachment 688413 [details] [diff] [review]
Replace HTMLElement quickstubs with new binding methods v1

Review of attachment 688413 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsGenericHTMLElement.cpp
@@ +1896,5 @@
> +                                                                              \
> +  return nsINode::SetOn##name_(handler, error);                               \
> +}
> +#include "nsEventNameList.h"
> +#undef ERROR_EVENT  

Trailing whitespace

::: dom/base/nsGlobalWindow.cpp
@@ +11330,5 @@
>  #include "nsEventNameList.h"
>  #undef TOUCH_EVENT
>  #undef WINDOW_ONLY_EVENT
> +#undef BEFOREUNLOAD_EVENT  
> +#undef ERROR_EVENT  

And here

::: dom/base/nsGlobalWindow.h
@@ +673,5 @@
> +#include "nsEventNameList.h"
> +#undef TOUCH_EVENT
> +#undef WINDOW_ONLY_EVENT
> +#undef BEFOREUNLOAD_EVENT  
> +#undef ERROR_EVENT  

And here
(Assignee)

Comment 10

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0e79206a110
https://hg.mozilla.org/integration/mozilla-inbound/rev/92187bc8ec48
https://hg.mozilla.org/integration/mozilla-inbound/rev/f23cff8ec6c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5179738abfe
https://hg.mozilla.org/mozilla-central/rev/a0e79206a110
https://hg.mozilla.org/mozilla-central/rev/92187bc8ec48
https://hg.mozilla.org/mozilla-central/rev/f23cff8ec6c9
https://hg.mozilla.org/mozilla-central/rev/d5179738abfe
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

4 years ago
Depends on: 819751
Depends on: 819745
Depends on: 833518
You need to log in before you can comment on or make changes to this bug.