710546 - Server sent events Once the end of the file is reached, any pending data must be discarded

Status: RESOLVED FIXED

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

Description

[4esn0k]

User Agent: Opera/9.80 (Windows NT 6.0; U; Edition Next; ru) Presto/2.10.238 Version/12.00

Steps to reproduce:


according to latest spec:
Once the end of the file is reached, any pending data must be discarded. (If the file ends in the middle of an event, before the final empty line, the incomplete event is not dispatched.) - see http://dev.w3.org/html5/eventsource/


Actual results:

  var es = new EventSource('events.php');
  es.onmessage = function (e) {
    alert(e.data + ' ' + e.lastEventId);// Firefox alerts "data0 123"
  };

events.php:
   echo "id: 123\n";
    echo "data: data0";
    exit();



Expected results:

If the file ends in the middle of an event, before the final empty line, the incomplete event is not dispatched.
Also the last id field value received should be stored in a buffer and set as the EventSource's lastEventId only when the event is actually dispatched - see http://www.w3.org/Bugs/Public/show_bug.cgi?id=13761

Comment 1

[Wellington Fernando de Macedo]

Just a commentary for clarifying future questions. According to spec:
>> event         = *( comment / field ) end-of-line
>> field         = 1*name-char [ colon [ space ] *any-char ] end-of-line
>> end-of-line   = ( cr lf / cr / lf )
>> 
>> If the file ends in the middle of an event, before the final empty line, the incomplete 
>> event is not dispatched.

So, for instance, the following code won't dispatch an event either:
>> echo "data: data0\r";
>> exit;
But this one will:
>> echo "data: data0\r\r";
>> exit;

Comment 2

[Wellington Fernando de Macedo]

Attachment: v1

Comment 3

[Wellington Fernando de Macedo]

Comment on attachment 617246 [details] [diff] [review]
v1

Olli, just a comment, I've removed the test which was testing an invalid NCName char in the event field name because it has become obsolete due changes in the spec.

Comment 4

[Wellington Fernando de Macedo]

https://hg.mozilla.org/try/rev/e135d9bb9f35

Comment 5

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

(Hopefully reviewing this tomorrow.)

Comment 6

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

(In reply to Wellington Fernando de Macedo from comment #1)
> Just a commentary for clarifying future questions. According to spec:
> >> event         = *( comment / field ) end-of-line
> >> field         = 1*name-char [ colon [ space ] *any-char ] end-of-line
> >> end-of-line   = ( cr lf / cr / lf )
> >> 
> >> If the file ends in the middle of an event, before the final empty line, the incomplete 
> >> event is not dispatched.
> 
> So, for instance, the following code won't dispatch an event either:
> >> echo "data: data0\r";
> >> exit;
> But this one will:
> >> echo "data: data0\r\r";
> >> exit;
Er, what. Do we need two new lines? that looks like a spec bug to me.

Comment 7

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

Comment on attachment 617246 [details] [diff] [review]
v1

Actually, perhaps it is ok.

Have you tested other browsers?

Comment 8

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

Comment on attachment 617246 [details] [diff] [review]
v1


>+++ b/content/base/test/badContentType.eventsource
>@@ -1,3 +1,5 @@
> retry:500
> event: message
> data: 1
>+
>+

Actually, why do you need 2 empty lines? Shouldn't 1 be enough

Comment 9

[Wellington Fernando de Macedo]

>> Actually, why do you need 2 empty lines? Shouldn't 1 be enough
Actually, I think 1 empty line would be enough. But, according to the spec:

1. event         = *( comment / field ) end-of-line
2. field         = 1*name-char [ colon [ space ] *any-char ] end-of-line

Hence:
event = 1*name-char [ colon [ space ] *any-char ] end-of-line end-of-line

Also:
>> If the file ends in the *middle of an event*, *before the final* empty line,
>> the incomplete event is not dispatched.

>> Have you tested other browsers?
No, unfortunately I didn't. Anyway, if they dispatch with just one empty line then either the spec needs to be fixed or the other browsers implementations need to be fixed.

Comment 10

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

But you have the end-of-line after data: 1 and then 2 end-of-lines, so totally 3.
I think 2 should be enough, per spec

Comment 11

[Wellington Fernando de Macedo]

Humm, I think there are 2 end-of-lines, not three:
> data: 1 LF
>+ LF
>+ EOF

isn't it?

Comment 12

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

Um, right, silly me.
I interpreted the diff in a wrong way.

Comment 13

[4esn0k]

any updates?

Comment 14

[Wellington Fernando de Macedo]

It is already done. Perhaps it needs just to be update to the trunk.

Comment 15

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

Argh, this wasn't just pushed to m-c. My bad.
Wellington, want to update the patch to work with trunk? If you don't have time, I can do it.

Comment 16

[Wellington Fernando de Macedo]

Ok, no problem Olli. I'll update it tomorrow and upload again here.

Comment 17

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

Thanks!

Comment 18

[Wellington Fernando de Macedo]

Attachment: v1 updated

Comment 19

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

Comment on attachment 704732 [details] [diff] [review]
v1 updated

tree is closed atm, so can push this right now.

Comment 20

[4esn0k]

fine, thank you

Comment 21

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

Tryserver https://tbpl.mozilla.org/?tree=Try&rev=e1a689a9390b
Waiting for the results before pushing.

Comment 22

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

https://hg.mozilla.org/mozilla-central/rev/653199a8edea