Last Comment Bug 710546 - Server sent events Once the end of the file is reached, any pending data must be discarded
: Server sent events Once the end of the file is reached, any pending data must...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: x86 Windows Vista
: -- normal with 1 vote (vote)
: ---
Assigned To: Wellington Fernando de Macedo
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-13 21:59 PST by 4esn0k
Modified: 2013-01-22 12:58 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (13.19 KB, patch)
2012-04-21 12:14 PDT, Wellington Fernando de Macedo
bugs: review+
Details | Diff | Splinter Review
v1 updated (13.18 KB, patch)
2013-01-21 17:50 PST, Wellington Fernando de Macedo
bugs: review+
bugs: checkin+
Details | Diff | Splinter Review

Description 4esn0k 2011-12-13 21:59:22 PST
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 2012-04-21 09:47:18 PDT
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 2012-04-21 12:14:12 PDT
Created attachment 617246 [details] [diff] [review]
v1
Comment 3 Wellington Fernando de Macedo 2012-04-21 12:17:54 PDT
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 2012-04-21 12:37:52 PDT
https://hg.mozilla.org/try/rev/e135d9bb9f35
Comment 5 Olli Pettay [:smaug] 2012-04-23 13:00:25 PDT
(Hopefully reviewing this tomorrow.)
Comment 6 Olli Pettay [:smaug] 2012-04-26 04:59:06 PDT
(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] 2012-04-26 05:05:02 PDT
Comment on attachment 617246 [details] [diff] [review]
v1

Actually, perhaps it is ok.

Have you tested other browsers?
Comment 8 Olli Pettay [:smaug] 2012-04-26 06:43:33 PDT
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 2012-04-26 18:13:36 PDT
>> 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] 2012-04-27 04:45:07 PDT
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 2012-04-27 17:30:23 PDT
Humm, I think there are 2 end-of-lines, not three:
> data: 1 LF
>+ LF
>+ EOF

isn't it?
Comment 12 Olli Pettay [:smaug] 2012-04-27 17:35:48 PDT
Um, right, silly me.
I interpreted the diff in a wrong way.
Comment 13 4esn0k 2013-01-18 21:21:38 PST
any updates?
Comment 14 Wellington Fernando de Macedo 2013-01-20 06:26:40 PST
It is already done. Perhaps it needs just to be update to the trunk.
Comment 15 Olli Pettay [:smaug] 2013-01-20 14:34:26 PST
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 2013-01-20 15:20:30 PST
Ok, no problem Olli. I'll update it tomorrow and upload again here.
Comment 17 Olli Pettay [:smaug] 2013-01-20 15:42:49 PST
Thanks!
Comment 18 Wellington Fernando de Macedo 2013-01-21 17:50:50 PST
Created attachment 704732 [details] [diff] [review]
v1 updated
Comment 19 Olli Pettay [:smaug] 2013-01-22 04:29:04 PST
Comment on attachment 704732 [details] [diff] [review]
v1 updated

tree is closed atm, so can push this right now.
Comment 20 4esn0k 2013-01-22 06:24:48 PST
fine, thank you
Comment 21 Olli Pettay [:smaug] 2013-01-22 10:18:50 PST
Tryserver https://tbpl.mozilla.org/?tree=Try&rev=e1a689a9390b
Waiting for the results before pushing.
Comment 22 Olli Pettay [:smaug] 2013-01-22 12:35:40 PST
https://hg.mozilla.org/mozilla-central/rev/653199a8edea

Note You need to log in before you can comment on or make changes to this bug.