w3c / geolocation

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

Remove [NoInterfaceObject] #19

Closed annevk closed 5 years ago

annevk commented 5 years ago

Carefully consider with what name to expose these interfaces and then please drop this. We're going to remove it from IDL per https://github.com/heycam/webidl/issues/430 and https://github.com/heycam/webidl/pull/609.

anssiko commented 5 years ago

@annevk, as you guessed, this spec is not maintained. That said, let's try to find out a way, using errata or some such process, to get this updated. Pinging @dontcallmedom as the good first contact.

All - spec maintenance issue aside, Geolocation, Coordinates, Position, PositionError, are the current names that are not exposed. These names seem a bit too generic to me to be exposed as is and may create problems in the future with names clashing. For example, the active spec effort exposes GeolocationSensor constructor.

I'd suggest prefixing Geolocation interface with its host interface's name, rename to NavigatorGeolocation. This seems to align with the established convention. As for the rest, maybe use the same prefix? NavigatorGeolocationCoordinates etc.?

Suggestions?

kenchris commented 5 years ago

As we want to deprecated this spec and replace it with a new one based on Generic Sensors, it seems unfortunate to expose these now @reillyeon @slightlyoff

annevk commented 5 years ago

I think exposing the interface is really the least of your problems with trying to replace this API.

reillyeon commented 5 years ago

I agree with the suggestion to rename the interfaces to make it clear they are Geolocation-specific. Perhaps even adding a prefix or suffix like "Legacy" to distinguish from any types related to the new GeolocationSensor work.

anssiko commented 5 years ago

I don't recall any interfaces exposed with "Legacy" suffix. WebIDL has some extended attributes with that name, but they're not web-exposed.

It is realistic to expect that the "legacy API" will ship in parallel with the new API for a very long time until its usage falls below a certain threshold, so maybe "Legacy" suffix is not appropriate.

To make it easier to review, here's the complete proposal that removes [NoInterfaceObject]s and adds NavigatorGeolocation prefixes on exposed interfaces:

partial interface Navigator {
  readonly attribute NavigatorGeolocation geolocation;
};

interface NavigatorGeolocation {
  void getCurrentPosition(PositionCallback successCallback,
                          optional PositionErrorCallback errorCallback,
                          optional PositionOptions options);

  long watchPosition(PositionCallback successCallback,
                     optional PositionErrorCallback errorCallback,
                     optional PositionOptions options);

  void clearWatch(long watchId);
};

callback PositionCallback = void (NavigatorGeolocationPosition position);

callback PositionErrorCallback = void (NavigatorGeolocationPositionError positionError);

dictionary PositionOptions {
  boolean enableHighAccuracy = false;
  [Clamp] unsigned long timeout = 0xFFFFFFFF;
  [Clamp] unsigned long maximumAge = 0;
};

interface NavigatorGeolocationPosition {
  readonly attribute NavigatorGeolocationCoordinates coords;
  readonly attribute DOMTimeStamp timestamp;
};

interface NavigatorGeolocationCoordinates {
  readonly attribute double latitude;
  readonly attribute double longitude;
  readonly attribute double? altitude;
  readonly attribute double accuracy;
  readonly attribute double? altitudeAccuracy;
  readonly attribute double? heading;
  readonly attribute double? speed;
};

interface NavigatorGeolocationPositionError {
  const unsigned short PERMISSION_DENIED = 1;
  const unsigned short POSITION_UNAVAILABLE = 2;
  const unsigned short TIMEOUT = 3;
  readonly attribute unsigned short code;
  readonly attribute DOMString message;
};
reillyeon commented 5 years ago

"NavigatorGeolocation" prefix looks good to me. There are a few more renames required for consistency in the IDL snippet above.

anssiko commented 5 years ago

Fixed the IDL, maybe I managed to update all the occurrences now. Editing a bigger block of text on mobile is hard it seems.

xfq commented 5 years ago

@anssiko About the spec maintenance issue, I think one possible plan is to bring it to the DAS WG and publish a new version. WDYT?

(This should be discussed in a separate issue, tho.)

anssiko commented 5 years ago

(@xfq, thanks, please feel free to open a separate issue for that. I assume the charter repo is the right place?)

xfq commented 5 years ago

Just filed w3c/dap-charter#76 for the spec maintenance issue.

anssiko commented 5 years ago

(Update: W3M is now working on making maintenance of this spec possible, tracked in https://github.com/w3c/dap-charter/issues/76.)

anssiko commented 5 years ago

We got a W3M approval to land a fix to this issue.

PR at https://github.com/w3c/geolocation-api/pull/20