Fix minor Server-Sent Events issues

RESOLVED WONTFIX

Status

()

Core
DOM
RESOLVED WONTFIX
7 years ago
5 years ago

People

(Reporter: Wellington Fernando de Macedo, Assigned: Wellington Fernando de Macedo)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

2.08 KB, patch
Wellington Fernando de Macedo
: review-
Details | Diff | Splinter Review
9.84 KB, text/plain
Details
(Assignee)

Description

7 years ago
Created attachment 537373 [details]
test results

Opera has tests which fail in our implementation. The attachment compares the results of those tests when running them on Opera, Chrome and our implementation.
(Assignee)

Updated

7 years ago
Assignee: nobody → wfernandom2004
Status: NEW → ASSIGNED
(Assignee)

Comment 1

7 years ago
Created attachment 546326 [details] [diff] [review]
patch v1
Attachment #546326 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 2

7 years ago
Created attachment 546327 [details]
test results

Only request-status-error.htm is failing. However I think our implementation is correct.
Attachment #537373 - Attachment is obsolete: true

Comment 3

7 years ago
Could you perhaps attach the test results in some other form?

Comment 4

7 years ago
Comment on attachment 546326 [details] [diff] [review]
patch v1

>diff --git a/content/base/src/nsEventSource.cpp b/content/base/src/nsEventSource.cpp
>--- a/content/base/src/nsEventSource.cpp
>+++ b/content/base/src/nsEventSource.cpp
>@@ -475,16 +475,25 @@ nsEventSource::StreamReaderFunc(nsIInput
> 
>   PRInt32 srcCount, outCount;
>   PRUnichar out[2];
>   nsresult rv;
> 
>   const char *p = aFromRawSegment,
>              *end = aFromRawSegment + aCount;
> 
>+  // If we have more than one BOM char at the begin of the stream then
beginning of...
>+  // the converter will leave only one. So we handle the first BOM here instead
>+  // of inside nsEventSource::ParseCharacter(). See bug 662073.
>+  if (thisObject->mStatus == PARSE_STATE_BEGIN_OF_STREAM && aCount >= 2 &&
>+      static_cast<PRUnichar>(*p) == BOM_CHAR) {
>+    thisObject->mStatus = PARSE_STATE_BOM_WAS_READ;  // ignore it
>+    p += 2;
>+  }
>+

r=me if you add tests.
Attachment #546326 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 5

7 years ago
> r=me if you add tests.
Perhaps I could rewrite the Opera tests to Mochitest. Do you still have the url address of the Opera's php test sources?
(Assignee)

Comment 6

7 years ago
Also, because I'm not an expert on laws, do we have license to rewrite the tests?

Comment 7

7 years ago
http://tc.labs.opera.com/apis/EventSource/
http://tc.labs.opera.com/apis/EventSource/resources/

I'll ask about the license of those tests.

Comment 8

7 years ago
Anne said it is ok to reuse the tests. No need to add any license text.
(Assignee)

Comment 9

7 years ago
Created attachment 546456 [details]
test results
Attachment #546327 - Attachment is obsolete: true
(Assignee)

Comment 10

7 years ago
> Anne said it is ok to reuse the tests. No need to add any license text.
Please, ask them to add some written text in the sources that explicitly states which license the sources are under.
asking...
(Assignee)

Updated

5 years ago
Attachment #546326 - Flags: review+ → review-
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 12

5 years ago
Duplicated of bugs 869432, 869865, 869874, 869875, 869878 and 869882.
You need to log in before you can comment on or make changes to this bug.