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

Description Adam Bergkvist 2010-01-05 07:49:06 PST

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

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

Created attachment 45891 [details]
patch

Proposed patch.

Comment 2 WebKit Review Bot 2010-01-05 08:00:32 PST

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

Comment 3 Darin Adler 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.

Comment 4 Eric Seidel (no email) 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.

Comment 5 Adam Bergkvist 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

Comment 6 WebKit Review Bot 2010-01-07 08:38:37 PST

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

Comment 7 Eric Seidel (no email) 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.

Comment 8 Adam Barth 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.

Comment 9 WebKit Commit Bot 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>

Comment 10 WebKit Commit Bot 2010-03-15 21:59:09 PDT

All reviewed patches have been landed.  Closing bug.