IDL/API surface review

[domenic]

In #48 @yoavweiss asked me to do a check on the IDL. Here's a general issue, that mixes in a bit of API review too.

• NavigatorUABrandVersionDict is a bit of a strange name, mainly because it includes Dict. This is not web-exposed so it doesn't really matter, but I'd suggest NavigatorUAPlatform.

• required on NavigatorUABrandVersionDict's brand doesn't make sense here, since this is only ever returned, not accepted, by Web IDL APIs. Better to just omit that.

• Why is the array of NavigatorUABrandVersionDicts titled brand? So you'd access the brand via navigator.brand[0].brand? If it's an array, it should have a plural name. And I think a "brand + version" is a "platform" elsewhere, so probably this should be platforms?

• getPlatform() might be clearer as getRealPlatform() or something.

• navigator.getUserAgent() should no longer be a function, but instead a readonly attribute, since it's no longer promise-returning or expensive. Which brings up some name collision issues, since the natural name is navigator.userAgent... Maybe just self.userAgent? Tricky.

• The processing model is totally wrong. getUserAgent() is said to return a promise, but it does not in the IDL. The methods and getters are not specced to have appropriate steps for running them; instead they are somehow "initialized" (I guess this means that instead of navigator.getUserAgent().getPlatform being a function, it got overwritten, per the spec, with the platform brand and version string?)

[yoavweiss]

Thanks for the review!!

Finally getting back to this with #70:

• NavigatorUABrandVersionDict is a bit of a strange name, mainly because it includes Dict. This is not web-exposed so it doesn't really matter, but I'd suggest NavigatorUAPlatform.

Removed the "Dict" part. I'm afraid that "platform" would be overloaded, as we use it here for OS (and have since split the OS brand from version).
Open to renaming tho.

• required on NavigatorUABrandVersionDict's brand doesn't make sense here, since this is only ever returned, not accepted, by Web IDL APIs. Better to just omit that.

Omitted.

• Why is the array of NavigatorUABrandVersionDicts titled brand? So you'd access the brand via navigator.brand[0].brand? If it's an array, it should have a plural name. And I think a "brand + version" is a "platform" elsewhere, so probably this should be platforms?

That's indeed silly. Renamed to UASet

• getPlatform() might be clearer as getRealPlatform() or something.

Changed the API shape, so not sure this is still relevant

• navigator.getUserAgent() should no longer be a function, but instead a readonly attribute, since it's no longer promise-returning or expensive. Which brings up some name collision issues, since the natural name is navigator.userAgent... Maybe just self.userAgent? Tricky.

Renamed to userAgentData. Let me know if that works from your perspective

• The processing model is totally wrong. getUserAgent() is said to return a promise, but it does not in the IDL. The methods and getters are not specced to have appropriate steps for running them; instead they are somehow "initialized" (I guess this means that instead of navigator.getUserAgent().getPlatform being a function, it got overwritten, per the spec, with the platform brand and version string?)

Yeah, apologies for that. I modified the API shape without changing processing along with it. I now revamped it. Let me know what you think on the PR.

[yoavweiss]

Closing as #70 landed