Bug 80483

Summary: Deprecate ondisplay()
Product: WebKit Reporter: Jon Lee <jonlee>
Component: DOMAssignee: Jon Lee <jonlee>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, atwilson, dglazkov, jberlin, jianli, ojan, sam, webkit-bug-importer, webkit.review.bot, yael
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.7   
Bug Depends on: 80497    
Bug Blocks: 80472    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch jianli: review+

Jon Lee
Reported 2012-03-06 21:13:31 PST
<rdar://problem/10912432>
Attachments
Patch (11.93 KB, patch)
2012-03-07 16:08 PST, Jon Lee 		no flags
DetailsFormatted DiffDiff
Patch (13.21 KB, patch)
2012-03-08 00:37 PST, Jon Lee 		no flags
DetailsFormatted DiffDiff
Patch (13.21 KB, patch)
2012-03-10 12:35 PST, Jon Lee 		no flags
DetailsFormatted DiffDiff
Patch (3.90 KB, patch)
2012-03-13 15:57 PDT, Jon Lee 		jianli: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jon Lee
Comment 1 2012-03-07 16:08:03 PST
Created attachment 130715 [details] Patch
Jon Lee
Comment 2 2012-03-07 19:52:14 PST
The patch only deprecates ondisplay() if ENABLE(LEGACY_NOTIFICATIONS) is false, but does change the tests to use onshow().
Yael
Comment 3 2012-03-07 19:53:36 PST
*** Bug 78144 has been marked as a duplicate of this bug. ***
WebKit Review Bot
Comment 4 2012-03-07 20:17:50 PST
Comment on attachment 130715 [details] Patch Attachment 130715 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11861111 New failing tests: fast/notifications/notifications-double-show.html fast/notifications/notifications-with-permission.html fast/notifications/notifications-display-close-events.html
WebKit Review Bot
Comment 5 2012-03-07 21:05:33 PST
Comment on attachment 130715 [details] Patch Attachment 130715 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11866111 New failing tests: fast/notifications/notifications-double-show.html fast/notifications/notifications-with-permission.html fast/notifications/notifications-display-close-events.html
Jon Lee
Comment 6 2012-03-08 00:37:47 PST
Created attachment 130790 [details] Patch Attempt to fix cr bot
Jon Lee
Comment 7 2012-03-10 12:35:41 PST
Created attachment 131183 [details] Patch
Jon Lee
Comment 8 2012-03-10 14:00:58 PST
The current spec states that the proper name of the callback when the notification is queued or presented is onshow(), not ondisplay(): http://dev.w3.org/2006/webapi/WebNotifications/publish/Notifications.html
Jon Lee
Comment 9 2012-03-13 15:57:12 PDT
Created attachment 131738 [details] Patch
Jian Li
Comment 10 2012-03-14 17:21:58 PDT
What are you going to deal with ondisplay and onshow in the test files?
Jon Lee
Comment 11 2012-03-14 17:41:25 PDT
(In reply to comment #10) > What are you going to deal with ondisplay and onshow in the test files? There is no support right now for notifications on the mac, so I've opted at this point to leave these tests alone. Once we add support (bug 77969), I will have to duplicate the tests using the new API (bug 81048).
Jian Li
Comment 12 2012-03-15 11:16:38 PDT
(In reply to comment #11) > (In reply to comment #10) > > What are you going to deal with ondisplay and onshow in the test files? > > There is no support right now for notifications on the mac, so I've opted at this point to leave these tests alone. Once we add support (bug 77969), I will have to duplicate the tests using the new API (bug 81048). Then we will have to maintain 2 sets of tests that different port needs to choose among them. For now, I do not know if there is any better solution for this. We can come to this later.
Jon Lee
Comment 13 2012-03-15 15:44:16 PDT
Committed r110899: <http://trac.webkit.org/changeset/110899>
Note You need to log in before you can comment on or make changes to this bug.