Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Commit 56c5368

Browse files
johnmellorCommit bot
authored and
Commit bot
committed
Stop requiring NotificationAction action and title to be non-empty
Improve spec compliance. Note that http/tests/notifications/serviceworkerregistration-document-actions-throw.html still tests that they are set (but it's no longer possible to have an equivalent check in NotificationDataTest, since required WebIDL attributes are enforced at the V8 level by [1], and are bypassed when creating IDL objects from C++). [1]: https://chromium.googlesource.com/chromium/src/+/dfd8ec1b3edb7f876235bec1138ecec4f2da748d/third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp#55 BUG=557624 Review URL: https://codereview.chromium.org/1487063002 Cr-Commit-Position: refs/heads/master@{#362469}
1 parent e11a131 commit 56c5368

File tree

3 files changed

+1
-82
lines changed

3 files changed

+1
-82
lines changed

third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -48,40 +48,6 @@
4848
}).catch(unreached_rejection(test));
4949

5050
}, 'showNotification() must reject if a NotificationAction has no title.');
51-
52-
async_test(function(test) {
53-
var scope = 'resources/scope/' + location.pathname + "/emptyaction";
54-
55-
getActiveServiceWorkerWithMessagePort(test, script, scope).then(function(workerInfo) {
56-
assert_inherits(workerInfo.registration, 'showNotification', 'showNotification() must be exposed.');
57-
58-
workerInfo.registration.showNotification('Title', {
59-
actions: [{ action: "", title: "Foo" }]
60-
}).then(unreached_fulfillment(test)).catch(function(error) {
61-
assert_equals(error.name, 'TypeError');
62-
assert_equals(error.message, "Failed to execute 'showNotification' on 'ServiceWorkerRegistration': NotificationAction `action` must not be empty.");
63-
test.done();
64-
});
65-
}).catch(unreached_rejection(test));
66-
67-
}, 'showNotification() must reject if a NotificationAction has an empty action.');
68-
69-
async_test(function(test) {
70-
var scope = 'resources/scope/' + location.pathname + "/emptytitle";
71-
72-
getActiveServiceWorkerWithMessagePort(test, script, scope).then(function(workerInfo) {
73-
assert_inherits(workerInfo.registration, 'showNotification', 'showNotification() must be exposed.');
74-
75-
workerInfo.registration.showNotification('Title', {
76-
actions: [{ action: "foo", title: "" }]
77-
}).then(unreached_fulfillment(test)).catch(function(error) {
78-
assert_equals(error.name, 'TypeError');
79-
assert_equals(error.message, "Failed to execute 'showNotification' on 'ServiceWorkerRegistration': NotificationAction `title` must not be empty.");
80-
test.done();
81-
});
82-
}).catch(unreached_rejection(test));
83-
84-
}, 'showNotification() must reject if a NotificationAction has an empty title.');
8551
</script>
8652
</body>
8753
</html>

third_party/WebKit/Source/modules/notifications/NotificationData.cpp

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,8 @@ WebNotificationData createWebNotificationData(ExecutionContext* executionContext
7575

7676
const size_t maxActions = Notification::maxActions();
7777
for (const NotificationAction& action : options.actions()) {
78-
if (action.action().isEmpty()) {
79-
exceptionState.throwTypeError("NotificationAction `action` must not be empty.");
80-
return WebNotificationData();
81-
}
82-
83-
if (action.title().isEmpty()) {
84-
exceptionState.throwTypeError("NotificationAction `title` must not be empty.");
85-
return WebNotificationData();
86-
}
87-
8878
if (actions.size() >= maxActions)
89-
continue;
79+
break;
9080

9181
WebNotificationAction webAction;
9282
webAction.action = action.action();

third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -179,43 +179,6 @@ TEST_F(NotificationDataTest, DirectionValues)
179179
}
180180
}
181181

182-
TEST_F(NotificationDataTest, RequiredActionProperties)
183-
{
184-
NotificationOptions options;
185-
186-
// The NotificationAction.action property is required.
187-
{
188-
NotificationAction action;
189-
action.setTitle(kNotificationActionTitle);
190-
191-
HeapVector<NotificationAction> actions;
192-
actions.append(action);
193-
194-
options.setActions(actions);
195-
196-
TrackExceptionState exceptionState;
197-
WebNotificationData notificationData = createWebNotificationData(executionContext(), kNotificationTitle, options, exceptionState);
198-
ASSERT_TRUE(exceptionState.hadException());
199-
EXPECT_EQ("NotificationAction `action` must not be empty.", exceptionState.message());
200-
}
201-
202-
// The NotificationAction.title property is required.
203-
{
204-
NotificationAction action;
205-
action.setAction(kNotificationActionAction);
206-
207-
HeapVector<NotificationAction> actions;
208-
actions.append(action);
209-
210-
options.setActions(actions);
211-
212-
TrackExceptionState exceptionState;
213-
WebNotificationData notificationData = createWebNotificationData(executionContext(), kNotificationTitle, options, exceptionState);
214-
ASSERT_TRUE(exceptionState.hadException());
215-
EXPECT_EQ("NotificationAction `title` must not be empty.", exceptionState.message());
216-
}
217-
}
218-
219182
TEST_F(NotificationDataTest, MaximumActionCount)
220183
{
221184
HeapVector<NotificationAction> actions;

0 commit comments

Comments
 (0)