859808 - nsIDOMMozCellBroadcastMessage ported to WebIDL

Status: RESOLVED DUPLICATE of bug 906398

(Core :: DOM: Core & HTML, defect)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: patch

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 735241 [details] [diff] [review]
patch

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

r=me with these comments.

::: dom/bindings/Bindings.conf
@@ +248,5 @@
>  }],
>  
> +'MozCellBroadcast': {
> +    'nativeType': 'mozilla::dom::CellBroadcast',
> +    'headerFile': 'mozilla/dom/CellBroadcast.h'

Shouldn't need to have headerFile.

::: dom/cellbroadcast/src/CellBroadcast.cpp
@@ +8,4 @@
>  #include "nsIDOMMozCellBroadcastEvent.h"
>  #include "nsIDOMMozCellBroadcastMessage.h"
>  #include "mozilla/Services.h"
>  #include "nsDOMClassInfo.h"

Do you still need this?

::: dom/cellbroadcast/src/CellBroadcast.h
@@ +42,5 @@
> +  {
> +    return GetOwner();
> +  }
> +
> +  virtual JSObject* WrapObject(JSContext* aCx, JSObject* aScope, bool* aTriedToWrap) MOZ_OVERRIDE;

aTriedToWrap?

@@ +44,5 @@
> +  }
> +
> +  virtual JSObject* WrapObject(JSContext* aCx, JSObject* aScope, bool* aTriedToWrap) MOZ_OVERRIDE;
> +
> +  inline JSObject* GetOnreceived(JSContext* aCx)

Use IMPL_EVENT_HANDLER

::: dom/webidl/MozCellBroadcast.webidl
@@ +5,5 @@
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +interface MozCellBroadcast : EventTarget {
> +  [TreatNonCallableAsNull, SetterThrows]
> +  attribute Function? onreceived;

This should just be

[SetterThrows]
attribute EventHandler onreceived;

Comment 3

[Andrea Marchesini [:baku]]

(In reply to :Ms2ger from comment #2)
> Comment on attachment 735241 [details] [diff] [review]
> patch
> 
> Review of attachment 735241 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with these comments.
> 
> ::: dom/bindings/Bindings.conf
> @@ +248,5 @@
> >  }],
> >  
> > +'MozCellBroadcast': {
> > +    'nativeType': 'mozilla::dom::CellBroadcast',
> > +    'headerFile': 'mozilla/dom/CellBroadcast.h'
> 
> Shouldn't need to have headerFile.

No, otherwise the header file should be called MozCellBroadcast.h

Wait :) The only wait to test this patch was to write it for b2g18. This is the reason why I don't have IMPL_EVENT_HANDLER, aTriedToWrap & C.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> No, otherwise the header file should be called MozCellBroadcast.h

No, because the headerFile is derived from the nativeType.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: m-c patch