- From: Ilya Grigorik <igrigorik@google.com>
- Date: Fri, 16 Jan 2015 15:36:07 -0800
- To: Domenic Denicola <d@domenic.me>
- Cc: "public-web-perf@w3.org" <public-web-perf@w3.org>, www-tag <www-tag@w3.org>
- Message-ID: <CADXXVKrNG=zRWJFvO=m2PQNhGLKqP=1J+YwbCojyHwG0aXEH8w@mail.gmail.com>
Hi Domenic. You're cc'ed on the other thread, but for the public record... Please take a look at: http://lists.w3.org/Archives/Public/public-web-perf/2015Jan/0021.html - new proposal that, I believe, should address all of the above points. ig On Wed, Jan 7, 2015 at 1:49 PM, Domenic Denicola <d@domenic.me> wrote: > Hi all! [www-tag in bcc] > > The TAG has meant for a while now to take a look at the Navigation Error > Logging spec. I couldn't find a bug tracker for Navigation Error Logging, > so I thought I'd send our comments to the list. They're overall pretty > minor things, and my apologies if any of them have been discussed to death > before. > > - First the positive feedback: the spec is well-motivated, with a good > introduction that explains the problem statement. As a web developer I > never even think of this problem, but now I am quite interested in solving > it. Also, the privacy and security section is especially great to see. > > - NavigationErrorEntry should have a constructor; otherwise it is hard for > developers to understand how instances of NavigationErrorEntry exist at > all. (The answer you have to explain to them is: there *is* a constructor, > but it takes a secret parameter which only the user agent knows the value > of, so whenever you as a web developer try to call it, you get a TypeError. > That is not a very good answer and should be reserved for objects that > really do contain complicated internal secrets, e.g. Window or > SubtleCrypto.) > > - I was curious why the "name" attribute was called "name" instead of > "url"? > > - As explained in [1], for URLs the IDL type USVString should be used, > instead of DOMString. This impacts NavigationErrorEntry.prototype.name's > return type and Performance.prototype.enableNavigationErrorReporting's > parameter type. > > - Boolean parameters are in general an antipattern (see e.g. [2]), and > besides, it is very strange to call > `navigator.performance.enableNavigationErrorLogging(false)` to *disable* > navigation error logging. Especially given that to disable navigation error > *reporting*, you just do > `navigator.performance.disableNavigationErrorReporting()`. I think it would > be better to just split the method into two methods? There is of course no > loss of expressive power, since eg. `n.p.eNEL(myBool)` could be rewritten > as `n.p[myBool ? "eNEL" : "dNEL"]()`, which is a fairly familiar pattern > from other web APIs. Alternately if there are strong feelings that this > kind of Boolean parameter is desired, a better name (drawing from ClassList > and its jQuery predecessor) would be "toggle". > > - The WebIDL is slightly invalid in that you are not allowed to simply say > `Promise myMethod()`; you must give the promise a type. So I would instead > say `Promise<sequence<NavigationErrorEntry>> getNavigationErrors()`. > > - A minor nit, but I find the typedef sequence <NavigationErrorEntry> > NavigationErrorEntryList more obfuscating than helpful; I had to hunt down > the definition, whereas I already know what a sequence<T> is. > > - The getNavigationErrors method should be sure to do expensive steps "in > parallel" [3] > > - The "Monotonic Clock" clock section is interesting. I'm not sure I > entirely understand it though. How do you expect to test this requirement? > How do you expect implementations to meet it? > > Thanks! > > [1]: https://url.spec.whatwg.org/#url-apis-elsewhere > [2]: http://ariya.ofilabs.com/2011/08/hall-of-api-shame-boolean-trap.html > [3]: http://www.w3.org/2001/tag/doc/promises-guide#explicit-async-steps > > >
Received on Friday, 16 January 2015 23:37:18 UTC