Bug 80483 - Deprecate ondisplay()
Summary: Deprecate ondisplay()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.7
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
: 78144 (view as bug list)
Depends on: 80497
Blocks: 80472
  Show dependency treegraph
 
Reported: 2012-03-06 21:13 PST by Jon Lee
Modified: 2012-03-15 15:44 PDT (History)
10 users (show)

See Also:


Attachments
Patch (11.93 KB, patch)
2012-03-07 16:08 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (13.21 KB, patch)
2012-03-08 00:37 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (13.21 KB, patch)
2012-03-10 12:35 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (3.90 KB, patch)
2012-03-13 15:57 PDT, Jon Lee
jianli: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2012-03-06 21:13:31 PST
<rdar://problem/10912432>
Comment 1 Jon Lee 2012-03-07 16:08:03 PST
Created attachment 130715 [details]
Patch
Comment 2 Jon Lee 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().
Comment 3 Yael 2012-03-07 19:53:36 PST
*** Bug 78144 has been marked as a duplicate of this bug. ***
Comment 4 WebKit Review Bot 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
Comment 5 WebKit Review Bot 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
Comment 6 Jon Lee 2012-03-08 00:37:47 PST
Created attachment 130790 [details]
Patch

Attempt to fix cr bot
Comment 7 Jon Lee 2012-03-10 12:35:41 PST
Created attachment 131183 [details]
Patch
Comment 8 Jon Lee 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
Comment 9 Jon Lee 2012-03-13 15:57:12 PDT
Created attachment 131738 [details]
Patch
Comment 10 Jian Li 2012-03-14 17:21:58 PDT
What are you going to deal with ondisplay and onshow in the test files?
Comment 11 Jon Lee 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).
Comment 12 Jian Li 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.
Comment 13 Jon Lee 2012-03-15 15:44:16 PDT
Committed r110899: <http://trac.webkit.org/changeset/110899>