Bug 33210

Summary:

EventSource should support receiving empty events and events with newline characters only

Product:

WebKit

Reporter:

Adam Bergkvist <adam.bergkvist>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
patch	eric: review-
updated patch	none

Adam Bergkvist

Reported 2010-01-05 07:49:06 PST

According to the updated specification, EventSource should support receiving empty events and events with newline characters only.

Attachments
patch (4.55 KB, patch) 2010-01-05 07:57 PST, Adam Bergkvist	eric: review-	Details Formatted Diff Diff
updated patch (5.45 KB, patch) 2010-01-07 08:34 PST, Adam Bergkvist	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Adam Bergkvist

Comment 1 2010-01-05 07:57:17 PST

Created attachment 45891 [details] patch Proposed patch.

WebKit Review Bot

Comment 2 2010-01-05 08:00:32 PST

style-queue ran check-webkit-style on attachment 45891 [details] without any errors.

Darin Adler

Comment 3 2010-01-05 08:34:37 PST

Comment on attachment 45891 [details] patch Are the two code changes here separate fixes? How do the code changes relate to the desired behavior? What is the removeLast for? I think the comments in the change log are too terse. They assume that it's obvious what is wrong here and why this fixes it. Let those things are not clear to me.

Eric Seidel (no email)

Comment 4 2010-01-05 12:25:01 PST

Comment on attachment 45891 [details] patch I agree with Darin, this change is unreviewable. It's not clear from the ChagneLog or from the code comments what you're trying to do here.

Adam Bergkvist

Comment 5 2010-01-07 08:34:28 PST

Created attachment 46054 [details] updated patch I agree that the ChangeLog message was insufficient. Here's an updated patch with more descriptive message in the ChangeLog. The diff of the specification change in question: http://html5.org/tools/web-apps-tracker?from=4078&to=4079

WebKit Review Bot

Comment 6 2010-01-07 08:38:37 PST

style-queue ran check-webkit-style on attachment 46054 [details] without any errors.

Eric Seidel (no email)

Comment 7 2010-02-17 15:12:18 PST

You mention the "updated specification" in the ChangeLog, but a link would be much more helpful. Especially one which points to the paragraph in question. I don't even know what specification is under discussion here.

Adam Barth

Comment 8 2010-03-08 10:39:22 PST

Comment on attachment 46054 [details] updated patch The code looks reasonable. I haven't validated that this behavior actually matches what the spec requires, but I'm trusting Adam here.

WebKit Commit Bot

Comment 9 2010-03-15 21:59:03 PDT

Comment on attachment 46054 [details] updated patch Clearing flags on attachment: 46054 Committed r56035: <http://trac.webkit.org/changeset/56035>

WebKit Commit Bot

Comment 10 2010-03-15 21:59:09 PDT

All reviewed patches have been landed. Closing bug.

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