Re: [w3c/push-api] Add Declarative Web Push (PR #385)

@martinthomson commented on this pull request.

I'd like to see more explanatory text attached to this sort of change. (In the spec, not the PR, for avoidance of doubt.)

From what I can see, a message is opportunistically parsed as JSON.  If it parses and there is a "web_push": 9001 (?!) attribute, the browser attempts to make a notification.  If that works, the notification is shown.

There is also a mutable attribute attached, which would allow the SW the option of intercepting the notification and tweaking it before it is shown.  That would somewhat negate the benefits from a purely declarative notification, so it seems an unnecessary feature (the app could save the "web_push": 9001 bytes and just make a notification for itself).

> +                          If a notification has been shown through {{showNotification()}} at this
+                          point, then abort these steps.

This seems non-ideal.  I don't know how this is supposed to work, but this requires a trap in `showNotification` to track, but you aren't monkey-patching that.  Does `preventDefault` not work?

> +            </li>
+            <li>
+              <p>
+                If <var>notification</var>'s [=notification/URL=] is null, then return failure.
+              </p>
+            </li>
+            <li>
+              <p>
+                Let <var>appBadge</var> be null.
+              </p>
+            </li>
+            <li>
+              <p>
+                If <var>message</var>["`app_badge`"] [=map/exists=] and
+                <var>message</var>["`app_badge`"] is an integer in the range 0 to
+                9223372036854775807, inclusive, then set <var>appBadge</var> to

2^63-1.  You could say as much and not have people break out a calculator.  Is there a constant in the badging part of the API that you can refer to instead?

Second, given that you define the value above with this range, do you need to add this check?

Finally, why?  I can't believe that any badging UX will ever show anything that large (most stop after 9 or 99 in my experience).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/push-api/pull/385#pullrequestreview-2261838876
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/push-api/pull/385/review/2261838876@github.com>