Bug 39782 - notifications: in display+close layout test, should wait for display to close
Summary: notifications: in display+close layout test, should wait for display to close
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: John Gregg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-26 13:48 PDT by John Gregg
Modified: 2010-06-03 16:19 PDT (History)
2 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description John Gregg 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.
Comment 1 John Gregg 2010-05-26 16:04:00 PDT
Created attachment 57177 [details]
Patch
Comment 2 David Levin 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).
Comment 3 Yael 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?
Comment 4 John Gregg 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.
Comment 5 Yael 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.
Comment 6 John Gregg 2010-05-27 11:20:11 PDT
Created attachment 57262 [details]
Patch
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2010-06-03 16:19:29 PDT
All reviewed patches have been landed.  Closing bug.