834165 - Implement BlobEvent

Status: RESOLVED FIXED

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

Description

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

MediaRecorder api require this to transfer the encoder/image data to jscontext.

idl:
BlobEvent interface

[Constructor(DOMString type, optional BlobEventInit blobInitDict)]
interface BlobEvent : Event {
    readonly attribute Blob data;
};

dictionary BlobEventInit : EventInit {
    Blob data;
};

Comment 1

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: IDL for BlobEvent

Comment 2

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: WIP blobEvent patch

Comment 3

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: WIP2 blobEvent patch

Hi Olli, 
We want to add the BlobEvent for MediaRecorder server side event and let jsContext can get the encode data, Can you help to review it?
Thanks a lot.

Comment 4

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

Comment on attachment 708967 [details] [diff] [review]
WIP2 blobEvent patch

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

::: dom/interfaces/events/nsIDOMBlobEvent.idl
@@ +17,5 @@
> +  /**
> +   * Custom blob data associated with this event.
> +   */
> +  readonly attribute nsIDOMBlob data;
> +  void initBlobEvent(in DOMString aType,

This should be [noscript].

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 708419 [details] [diff] [review]
IDL for BlobEvent

I don't know what is MediaRecorder. Is it from some specification?
And if so, does the spec really have BlobEvent, since that is quite odd name.
It feels too generic.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 708419 [details] [diff] [review]
IDL for BlobEvent

Er, missed the URL :)

Comment 7

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Hi OLLi, 
Here is the draft url for reference. 
https://dvcs.w3.org/hg/dap/raw-file/tip/media-stream-capture/RecordingProposal.html
BTW, this is just the draft version.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 708967 [details] [diff] [review]
WIP2 blobEvent patch

iniBlobEvent should be [noscript]

nsDOMFile.h should be nsIDOMFile.h, right?
You shouldn't need to add anything to dictionary_helper_gen.conf
event_impl_gen.conf.in should take care of the dictionary automatically.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

And forget comment 5. The event is coming from a "spec".

Comment 10

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Hi Olli, 
I try to remove the dictionary_helper_gen.conf but found this would cause build break on DictinaryHelper.cpp, show can't find nsIDOMBlob.h....
change to [noscript], the js can't access the data attribute. Did i miss something?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

[noscript] for initBlobEvent only.

could you just add nsIDOMFile.h to the special include and exclude nsIDOMBlob.h in the 
dictionary_helper_gen.conf.in but not add [ 'BlobEventInit', 'nsIDOMBlobEvent.idl' ] ?

Comment 12

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: add blobEvent patch1

Hi Olli, 
Please help to review that, 
try server result:
https://tbpl.mozilla.org/?tree=Try&rev=5449988b57d6

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 709565 [details] [diff] [review]
add blobEvent patch1


> 	nsIDOMCustomEvent.idl			\
> 	nsIDOMCompositionEvent.idl		\
> 	nsIDOMWheelEvent.idl			\
>+	nsIDOMBlobEvent.idl				\
Align \ properly


>+[scriptable, builtinclass, uuid(84293ee0-68f5-11e2-9906-cf63ba8c6e43)]
>+interface nsIDOMBlobEvent : nsIDOMEvent
>+{
>+  /**
>+   * Custom blob data associated with this event.
>+   */
>+  readonly attribute nsIDOMBlob data;
>+  [noscript]
>+  void initBlobEvent(in DOMString aType,
>+                        in boolean aCanBubble,
>+                        in boolean aCancelable,
>+                        in nsIDOMBlob aData);
align parameters and perhaps a newline before [noscript]

Comment 14

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: checkin patch

add r=smaug, 
correct align problem.

Comment 15

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: checkin-patch

fix conflict

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5474b831ece7

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/5474b831ece7

Comment 18

[Jean-Yves Perrier [:teoli]]

I noted that our implementation differs from the spec: the Blob itself is optional, it can be null, whereas the spec has it mandatory.

Beside this, documentation written:
https://developer.mozilla.org/en-US/docs/Web/API/BlobEvent
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/22