- From: Domenic Denicola <d@domenic.me>
- Date: Wed, 7 Jan 2015 21:49:47 +0000
- To: "public-web-perf@w3.org" <public-web-perf@w3.org>
- CC: www-tag <www-tag@w3.org>
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 Wednesday, 7 January 2015 21:50:20 UTC