IDL amendments and small misc issues. #271

andypaicu · 2017-11-23T10:52:23Z

Addresses:
#270
#268
#267
#265
#266

Also replaced an obsolete RFC and removed some random whitespaces

Preview | Diff

Merging

andypaicu · 2017-11-23T10:53:56Z

CC @annevk @sideshowbarker @smaug---- @domenic

annevk · 2017-11-23T10:55:54Z

Are you sure this is web-compatible? new SecurityPolicyViolationEvent("x") doesn't throw in Chrome today.

andypaicu · 2017-11-23T11:23:37Z

IMO, if it doesn't it should. I don't think a SPV event makes any sense without the required fields.

I would rather fix Chrome.

mikewest · 2017-11-23T12:46:55Z

index.src.html

-          <a>shadow-including root</a> is not |global|'s <a>associated
+          <a>shadow-including root</A policy defines allowed and restricted behaviors, and may be applied to a Window, {{WorkerGlobalScope}, or WorkletGlobalScope as described in §4.2.2 Initialize a global object’s CSP list.
+
+a> is not |global|'s <a>associated


This looks like a typo.

mikewest · 2017-11-23T12:47:38Z

index.src.html

+          :: The result of executing the <a>URL serializer</a> on |violation|'s
+             <a for="violation">source file</a>, with the `exclude fragment`
+             flag set if the |violation|'s <a for="violation">source file</a>
+             it not null and the `empty string` otherwise.


In this doc, we're generally putting backticks around null, and not around "empty string" ("the empty string" is fine).

mikewest · 2017-11-23T12:50:57Z

index.src.html

-        readonly    attribute long           lineNumber;
-        readonly    attribute long           columnNumber;
+        readonly    attribute unsigned long  lineNumber;
+        readonly    attribute unsigned long  columnNumber;


Is it worth deprecating these names, and adding the lineno and colno names @annevk suggested? Should be straightforward to add to Chrome's implementation, and I agree that it would be nice to standardize to some extent when possible.

I don't know how to deprecate attributes, I added new attributes and comments to the old ones. Does that make sense?

mikewest · 2017-11-23T12:55:52Z

index.src.html

@@ -1462,33 +1462,33 @@ <h3 id="violation-events">

    [Constructor(DOMString type, optional SecurityPolicyViolationEventInit eventInitDict)]


I wonder if we can get away with just dropping the constructor entirely. I can't imagine that there are that many folks actually using it, nor do I know what they'd be doing with it if they were... Maybe add some metrics to Chrome?

Main use case is testing code paths.

I'll add some metrics.

If we do drop the constructor, does the dictionary have any point anymore?

No, but I'm not convinced we should drop the constructor. All event classes have a constructor. I don't see why this one gets to be an exception.

All event classes have a constructor.

https://www.google.com/url?q=https://html.spec.whatwg.org/%23the-beforeunloadevent-interface :)

But sure, if events should have constructors, there's not much harm in keeping our implementation. It just seems somewhat useless. shrug

See whatwg/html#2574.

Zirro · 2017-11-23T18:45:51Z

As you deprecate lineNumber and columnNumber there might be a case for doing the same with the attributes ending in URI in favour of URL, as suggested here. Would doing that be within the scope of this pull request, or would it require further discussion?

andypaicu · 2017-11-24T10:36:34Z

I'm afraid I don't understand the suggestion for URI vs URL. The standard linked in that comment it talking about using the uppercase variant "URL" for compound names not about using URL instead of URI.

It's not difficult to change if we're deprecating other names but I don't know if there's any reason to.

Zirro · 2017-11-24T15:09:38Z

@andypaicu The point could be made clearer in that paragraph, but the idea is that variations of "URL" such as URI and IRI add more confusion than value for users. Therefore it is suggested that newer APIs use "URL" or "url" everywhere, instead of URI, IRI and so on.

@domenic Since you brought it up in the issue earlier, would you like to add anything?

mikewest · 2017-11-27T14:35:26Z

I think it's probably reasonable to poke at the *URI bits with the goal of renaming them to *URL. We could do that by deprecating the existing names, and adding new names. Probably best in a separate patch.

That seems worthwhile to do for the events. I'm less convinced that it's a good idea for the POSTed report, as doing the same aliasing would bloat the request size. Either way, I'd suggest that a separate issue/PR is the right thing to do.

andypaicu · 2017-11-28T09:01:37Z

I will create a different PR following Mike's suggestion for the name changes/deprecations.

Does this PR look reasonable?

mikewest

This looks reasonable to me. Can you add some tests for the behavior changes?

* 'master' of https://github.com/w3c/webappsec-csp: (209 commits) Fix a few typos (w3c#280) Introduce 'prefetch-src'. (w3c#283) Clarify navigation behavior for 'script-src'. Incorrect indentation of the navigation check algorithm. IDL amendments and small misc issues. (w3c#271) Regenerate HTMLs. Origin link. NoncedElement link. link up inline css issue (w3c#228) Replaced 'alias' with 'copy' for less ambiguity (w3c#273) Cleanup `global object` usage to make sense with `Documents` (w3c#254) Elements with duplicated attributes are not nonceable. s/not-example.com/example.org/ Linked testing policy and fixed a few links (w3c#263) Rebuild HTML. Fix linking errors to 'script-like' and 'applet'. Adds WorkletGlobalScope as a concept to CSP. (w3c#205) Slight correction of host matching description (w3c#251) Fixed ambigous grammar (w3c#250) Replace Request.type based logic with Request.destination (w3c#231) ...

andypaicu added 2 commits October 13, 2017 11:07

Merge pull request #1 from w3c/master

c7e86f4

Merging

Merge remote-tracking branch 'upstream/master' into HEAD

e7f628a

mikewest reviewed Nov 23, 2017

View reviewed changes

andypaicu added 5 commits November 27, 2017 16:05

Merge remote-tracking branch 'upstream/master' into HEAD

252a9b2

Small changes and fixes to IDLs, and replaced obsolete RFC5988

28e2f7c

Fixed type mismatch for policy and sourceFile when raising a SPV Event

987d9f2

Review changes

721a7c7

Removed the renamed properties in IDLs

d846aed

andypaicu force-pushed the misc-issues branch from bfc9a42 to d846aed Compare November 28, 2017 08:57

Merge remote-tracking branch 'upstream/master' into HEAD

24c27c9

mikewest approved these changes Dec 1, 2017

View reviewed changes

andypaicu added 2 commits December 12, 2017 12:37

Merge remote-tracking branch 'upstream/master' into HEAD

ad30d4a

Merge remote-tracking branch 'origin/master' into misc-issues

d291415

andypaicu merged commit 82aebd3 into w3c:master Dec 13, 2017

andypaicu deleted the misc-issues branch December 13, 2017 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IDL amendments and small misc issues. #271

IDL amendments and small misc issues. #271

andypaicu commented Nov 23, 2017 •

edited by pr-preview bot

Loading

andypaicu commented Nov 23, 2017

annevk commented Nov 23, 2017

andypaicu commented Nov 23, 2017

mikewest Nov 23, 2017

andypaicu Nov 23, 2017

mikewest Nov 23, 2017

andypaicu Nov 23, 2017

mikewest Nov 23, 2017

andypaicu Nov 23, 2017

mikewest Nov 23, 2017

annevk Nov 23, 2017

andypaicu Nov 23, 2017

annevk Nov 23, 2017

mikewest Nov 23, 2017

mikewest Nov 23, 2017

annevk Nov 23, 2017

Zirro commented Nov 23, 2017 •

edited

Loading

andypaicu commented Nov 24, 2017

Zirro commented Nov 24, 2017

mikewest commented Nov 27, 2017

andypaicu commented Nov 28, 2017

mikewest left a comment

		@@ -1462,33 +1462,33 @@ <h3 id="violation-events">

		[Constructor(DOMString type, optional SecurityPolicyViolationEventInit eventInitDict)]

IDL amendments and small misc issues. #271

IDL amendments and small misc issues. #271

Conversation

andypaicu commented Nov 23, 2017 • edited by pr-preview bot Loading

andypaicu commented Nov 23, 2017

annevk commented Nov 23, 2017

andypaicu commented Nov 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zirro commented Nov 23, 2017 • edited Loading

andypaicu commented Nov 24, 2017

Zirro commented Nov 24, 2017

mikewest commented Nov 27, 2017

andypaicu commented Nov 28, 2017

mikewest left a comment

Choose a reason for hiding this comment

andypaicu commented Nov 23, 2017 •

edited by pr-preview bot

Loading

Zirro commented Nov 23, 2017 •

edited

Loading