869432 - [eventsource] readyStage on eventsource:close() returns CONNECTING instead of CLOSED

Status: VERIFIED FIXED

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

Description

[tinazhao]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.10 (KHTML, like Gecko) Chrome/23.0.1262.0 Safari/537.10

Steps to reproduce:

Reproduce Step:
ServerSentEvent latest Editor's Draft 23 April 2013 (http://dev.w3.org/html5/eventsource/) test
- Test wiki: http://www.w3.org/wiki/Webapps/Interop/ServerSentEvents
- Test suite: https://github.com/w3c/web-platform-tests/tree/master/eventsource
- Test case: https://github.com/w3c/web-platform-tests/blob/master/eventsource/eventsource-close.htm


Actual results:

Actual results:

Test case failed: 
assert_equals(source.readyState, source.CLOSED, "closed readyState");
readyState returns CONNECTING


Expected results:

Expected results:

assert_equals(source.readyState, source.CLOSED, "closed readyState");
readyState returns CLOSED succesfully

Comment 1

[Paul Silaghi, QA [:pauly]]

How exactly can I see the issue here ?
Can you please provide some more specific steps to reproduce the problem ?
https://developer.mozilla.org/en/Bug_writing_guidelines

Comment 2

[Boris Zbarsky [:bzbarsky]]

Paul, for future reference isssues with DOM objects are DOM, not JS.

Olli, do you know who owns this code nowadays?

Comment 3

[Wellington Fernando de Macedo]

Hello Olli, how are you? I will take a look on this. Do you have information if this and the other six related bugs are due recent changes in the spec or if they are due wrong implementation of the initial spec?

Comment 4

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

Hi Wellington!
It would be great if you had time to look at this.
I was told the test suite can be run here http://w3c-test.org/web-platform-tests/master/eventsource/
We should try to follow the latest spec.

Comment 5

[Wellington Fernando de Macedo]

Hmmm, ok, I've found that, according the updated spec, when a 204 (No data) Http response is sent the EventSource shall close and stop reconnecting. Since the 204 http code is considered a successful request code, this is the root cause for the failure. However, it is not clear in the spec if in this case:
- the user agent should or not to announce the connection (i.e. to fire an open event);
- the user agent should or not to fail the connection (In my understanding, by reading the test code, the user agent is supposed to fire an error event and close the connection).

Comment 6

[Wellington Fernando de Macedo]

Attachment: patch

I tried to push in into the Try-Server, but unfortunately my account has been disabled due inactivity.

Comment 7

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

https://tbpl.mozilla.org/?tree=Try&rev=fa607f3dfe88

Comment 8

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

Ah, the bug summary is wrong. The failing assertion isn't about close().

Comment 9

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

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

r+ assuming good tryserver results.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Thanks for the patch!

One request, in the future, please make sure that for future patches, you have Mercurial configured to generate all the necessary commit information in the patch. It makes life easier for those landing on your behalf.
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd70bcf3f479

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dd70bcf3f479

Comment 13

[tinazhao]

Verified. The test case get PASS on Firefox: nightly 26.0a1 (2013-08-13).