w3c / geolocation

W3C Geolocation API
https://www.w3.org/TR/geolocation/
81 stars 56 forks source link

Rename NavigatorGeolocation* interfaces to Geolocation* #23

Closed foolip closed 5 years ago

foolip commented 5 years ago

They were prefixed in https://github.com/w3c/geolocation-api/pull/20 when removing [NoInterfaceObject], to avoid polluting the global namespace with generic interface names.

However, no other specs that add an object to navigator prefix the interface name with Navigator. Examples: https://storage.spec.whatwg.org/#api https://w3c.github.io/clipboard-apis/#navigator-interface https://w3c.github.io/keyboard-lock/#navigator-interface https://w3c.github.io/permissions/#navigator-and-workernavigator-extension https://w3c.github.io/presentation-api/#interface-presentation https://w3c.github.io/webappsec-credential-management/#framework-credential-management https://wicg.github.io/media-capabilities/#navigators-extensions https://wicg.github.io/mediasession/#the-mediasession-interface https://wicg.github.io/netinfo/#navigatornetworkinformation-interface

foolip commented 5 years ago

I found the examples from a merging of all IDL from all specs. Here's the raw text I was looking at for some more examples:

[Exposed=Window]
interface Navigator {
  Promise<BatteryManager> getBattery();
    boolean sendBeacon(USVString url, optional BodyInit? data = null);
  [SecureContext, SameObject] readonly attribute Clipboard clipboard;
  [SecureContext, SameObject] readonly attribute CredentialsContainer credentials;
    [SecureContext] Promise<MediaKeySystemAccess> requestMediaKeySystemAccess(DOMString keySystem,
                                                                              sequence<MediaKeySystemConfiguration> supportedConfigurations);
  sequence<Gamepad?> getGamepads();
   readonly attribute Geolocation geolocation;
  [SecureContext, SameObject] readonly attribute Keyboard keyboard;
  [SecureContext, SameObject] readonly attribute Keyboard keyboard;
  [SameObject] readonly attribute MediaCapabilities mediaCapabilities;
    [SameObject, SecureContext]
    readonly        attribute MediaDevices mediaDevices;
    [SecureContext]
    void getUserMedia(MediaStreamConstraints constraints, NavigatorUserMediaSuccessCallback successCallback, NavigatorUserMediaErrorCallback errorCallback);
  [SameObject] readonly attribute MediaSession mediaSession;
  readonly attribute Permissions permissions;
    readonly  attribute long maxTouchPoints;
  [SecureContext, SameObject] readonly attribute Presentation presentation;
  [SecureContext, SameObject] readonly attribute ServiceWorkerContainer serviceWorker;
    boolean vibrate(VibratePattern pattern);
  [SecureContext] Promise<WakeLock> getWakeLock(WakeLockType type);
  [SameObject]
  readonly attribute Bluetooth bluetooth;
  [SecureContext] Promise<void> share(optional ShareData data);
  [SecureContext] Promise<MIDIAccess> requestMIDIAccess(optional MIDIOptions options);
  [SameObject] readonly attribute USB usb;
  [SameObject] readonly attribute XR xr;
  readonly attribute DOMString appCodeName; // constant "Mozilla"
  readonly attribute DOMString appName; // constant "Netscape"
  readonly attribute DOMString appVersion;
  readonly attribute DOMString platform;
  readonly attribute DOMString product; // constant "Gecko"
  [Exposed=Window] readonly attribute DOMString productSub;
  readonly attribute DOMString userAgent;
  [Exposed=Window] readonly attribute DOMString vendor;
  [Exposed=Window] readonly attribute DOMString vendorSub;
  [Exposed=Window] boolean taintEnabled(); // constant false
  [Exposed=Window] readonly attribute DOMString oscpu;
  readonly attribute DOMString language;
  readonly attribute FrozenArray<DOMString> languages;
  readonly attribute boolean onLine;
  void registerProtocolHandler(DOMString scheme, USVString url, DOMString title);
  void unregisterProtocolHandler(DOMString scheme, USVString url);
  readonly attribute boolean cookieEnabled;
  [SameObject] readonly attribute PluginArray plugins;
  [SameObject] readonly attribute MimeTypeArray mimeTypes;
  boolean javaEnabled();
  readonly attribute unsigned long long hardwareConcurrency;
  readonly attribute NetworkInformation connection;
  readonly attribute StorageManager storage;
  readonly attribute boolean webdriver;
  // objects implementing this interface also implement the interfaces given below
};
foolip commented 5 years ago

As a drive-by I added [SameObject] and sent PRs to other specs too: https://github.com/whatwg/storage/pull/66 https://github.com/w3c/permissions/pull/190 https://github.com/WICG/netinfo/pull/79

anssiko commented 5 years ago

@foolip, thanks for paying attention to these details. This was discussed in #19 and the summary that motivated the proposal seems to be:

I'd like to hear what others think? This might not be a huge ergonomic issue, but is nice to get right.

reillyeon commented 5 years ago

@inexorabletash also pointed out to me that Object.prototype.toString(navigator.geolocation) currently returns "[object Geolocation]" and some tools, such as the Closure compiler, might depend on this behavior. This might mean that we have already lost the opportunity to rename these types.

reillyeon commented 5 years ago

Correction: Object.prototype.toString(navigator.geolocation) returns "[object Object]" on all the browsers I tested however navigator.geolocation.__proto__.toString() returns "[object Geolocation]" in Chrome and "[object GeolocationPrototype]" in Edge, Firefox and Safari.

inexorabletash commented 5 years ago

FWIW, the canonical incantation is Object.prototype.toString.call(navigator.geolocation) (note .call in there)

(I mis-typed in chat, sorry!)

foolip commented 5 years ago

Right, given an instance it is possible to get the prototype and the interface object event if the interface object isn't exposed. To take an example of an interface which is exposed, Object.getPrototypeOf(document.createAttribute('foo')) === Attr.prototype and Object.getPrototypeOf(document.createAttribute('foo')).constructor === Attr are both true.

I think the risks are quite low of a rename, but you can always search httparchive for the old names of the interfaces to get an idea.

Avoiding a common prefix with GeolocationSensor seems hard, is autocompletion the only reason to avoid it?

Maybe the best tradeoff is to just not expose the interfaces, thereby also avoiding https://github.com/w3c/geolocation-api/issues/25?

foolip commented 5 years ago

@annevk

annevk commented 5 years ago

It's not clear to me GeolocationSensor has cross-browser buy-in, so I don't think it should get to dictate what we do here. And autocomplete for a feature used at most once in a typical project doesn't strike me as worth optimizing for, especially as often you probably get the functionality via some library anyway.

reillyeon commented 5 years ago

I am okay with merging this. It also resolves an issue I discovered when implementing this in Chromium where some systems might be depending on the old name of the Geolocation interface.

Given that Sensor subclasses generally do not expose additional types for their reading values. I am not anticipating much overlap between this and GeolocationSensor aside from the autocomplete confusion mentioned above.

anssiko commented 5 years ago

I find @annevk’s arguments reasonable. Also considering Sensor subclasses only expose constructors limits clash potential. Let’s merge this unless someone shouts.

anssiko commented 5 years ago

@reillyeon, no concerns raised, so please feel free to merge this PR at will and coordinate with Blink's Intent to Implement and Ship.

Thanks everyone for your comments and @foolip for taking an action.

marcoscaceres commented 4 years ago

Landed in gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1575144 Thanks to @sidvishnoi!

anssiko commented 4 years ago

Thank you all! To summarize, this is now fixed in all major engines: