Bug 39782

Summary:

notifications: in display+close layout test, should wait for display to close

Product:

WebKit

Reporter:

John Gregg <johnnyg>

Component:

WebCore Misc.

Assignee:

John Gregg <johnnyg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, yael

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
Patch	none

John Gregg

Reported 2010-05-26 13:48:30 PDT

The notification spec says display is async; in this test we shouldn't try to close the notification before the display has fired.

Attachments
Patch (1.80 KB, patch) 2010-05-26 16:04 PDT, John Gregg	no flags	Details Formatted Diff Diff
Patch (2.73 KB, patch) 2010-05-27 11:20 PDT, John Gregg	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

John Gregg

Comment 1 2010-05-26 16:04:00 PDT

Created attachment 57177 [details] Patch

David Levin

Comment 2 2010-05-26 18:09:36 PDT

Comment on attachment 57177 [details] Patch > Index: LayoutTests/ChangeLog > + notifications: in display+close layout test, expect async events Your title in the bug is better than this (considering fixing this one to match the bug).

Yael

Comment 3 2010-05-27 06:52:56 PDT

Please note that there is one more issue with the layout tests. According to the spec at http://dev.w3.org/2006/webapi/WebNotifications/publish (8.2.2) "If the fetch algorithm returns error information, fire the error event on the notification object and stop executing this algorithm." Since the icon urls in the layout tests don't point to valid icons, almost all the tests should fire an error, or am I missing something?

John Gregg

Comment 4 2010-05-27 09:27:01 PDT

Yael, yes you are correct, it's just that the current test harness for notifications doesn't implement any loading, so I'm just verifying that the icon URL supplied gets passed to the NotificationPresenter. It would be nice to separate some of the other functionality from the icon loading, so in this case of testing the display and close events, I will change it to have no icon. It probably makes sense to do that for most of the tests, and then we can isolate the loading success and failure in different tests.

Yael

Comment 5 2010-05-27 10:00:05 PDT

(In reply to comment #4) > I will change it to have no icon. It probably makes sense to do that for most of the tests, and then we can isolate the loading success and failure in different tests. Thanks for making the change.

John Gregg

Comment 6 2010-05-27 11:20:11 PDT

Created attachment 57262 [details] Patch

WebKit Commit Bot

Comment 7 2010-06-03 16:19:24 PDT

Comment on attachment 57262 [details] Patch Clearing flags on attachment: 57262 Committed r60643: <http://trac.webkit.org/changeset/60643>

WebKit Commit Bot

Comment 8 2010-06-03 16:19:29 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.