Re: [w3c/manifest] Add IDL section and define IDL for all the members (#613)

mgiuca commented on this pull request.



> +             DOMString lang;
+             USVString name;
+             USVString short_name;
+             USVString description;
+             sequence<ImageResource> icons;
+             sequence<ImageResource> screenshots;
+             sequence<USVString> categories;
+             DOMString iarc_rating_id;
+             USVString start_url;
+             DisplayModeType display = "browser";
+             OrientationLockType orientation;
+             USVString theme_color;
+             USVString background_color;
+             USVString scope;
+             ServiceWorkerRegistration serviceworker;
+             sequence<ExternalApplicationResource> related_applications;

`= false` (then remove the equivalent sentence down in its section).

> -             USVString? short_name;
-             USVString? description;
-             sequence<ImageResource>? icons;
-             sequence<ImageResource>? screenshots;
-             sequence<USVString>? categories;
-             DOMString? iarc_rating_id;
-             USVString? start_url;
-             DisplayModeType? display;
-             OrientationLockType? orientation;
-             USVString? theme_color;
-             USVString? background_color;
-             USVString? scope;
-             ServiceWorkerRegistration? serviceworker;
-             sequence<ExternalApplicationResource>? related_applications;
-             boolean? prefer_related_applications;
+             TextDirectionType dir;

`= "auto"`

> -             sequence<ImageResource>? icons;
-             sequence<ImageResource>? screenshots;
-             sequence<USVString>? categories;
-             DOMString? iarc_rating_id;
-             USVString? start_url;
-             DisplayModeType? display;
-             OrientationLockType? orientation;
-             USVString? theme_color;
-             USVString? background_color;
-             USVString? scope;
-             ServiceWorkerRegistration? serviceworker;
-             sequence<ExternalApplicationResource>? related_applications;
-             boolean? prefer_related_applications;
+             TextDirectionType dir;
+             DOMString lang;
+             USVString name;

Thought bubble: This should possibly be `required` (though let's not change it right now because the existing algorithm doesn't require it, and I want to change the semantics as little as possible).

>            </li>
-          <li>Let <var>language</var> of <var>parsed manifest</var> be the
-          result of running the <a>steps for processing the <code>lang</code>
+          <li>Let <var>manifest["<a>display</a>"]</var> be the result of
+          running the <a>steps for post-processing the <code>display</code>

Up to you, but it seems it would actually make this patch smaller if you did it now (i.e., you would delete more lines without adding any).

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

Received on Wednesday, 4 October 2017 04:55:48 UTC