Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename NavigatorGeolocation* interfaces to Geolocation* #23

Merged
merged 2 commits into from
Feb 18, 2019
Merged

Rename NavigatorGeolocation* interfaces to Geolocation* #23

merged 2 commits into from
Feb 18, 2019

Conversation

@foolip
Copy link
Member Author

foolip commented Feb 14, 2019

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
Copy link
Member Author

foolip commented Feb 14, 2019

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

@anssiko
Copy link
Member

anssiko commented Feb 14, 2019

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

  • find a name that avoids confusing this now exposed interface with the GeolocationSensor constructor (think IDE autocomplete use case), and
  • make a clear distinction between interfaces associated with the Geolocation API (in maintenance) and the Geolocation Sensor (in active development, based on Generic Sensor API), also
  • a bunch of interface mixins are Navigator-prefixed but they are not exposed, so the argument I made for an established convention is moot if we scope this to exposed interfaces.

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

@reillyeon
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

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

(I mis-typed in chat, sorry!)

@foolip
Copy link
Member Author

foolip commented Feb 14, 2019

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 #25?

@foolip
Copy link
Member Author

foolip commented Feb 14, 2019

@annevk

@annevk
Copy link
Member

annevk commented Feb 15, 2019

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
Copy link
Member

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
Copy link
Member

anssiko commented Feb 15, 2019

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

@anssiko
Copy link
Member

anssiko commented Feb 18, 2019

@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.

@reillyeon reillyeon merged commit 5c51aa8 into w3c:gh-pages Feb 18, 2019
@foolip foolip deleted the no-navigator-prefix branch February 19, 2019 09:03
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 11, 2019
This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 11, 2019
This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f
aarongable pushed a commit to chromium/chromium that referenced this pull request Sep 11, 2019
This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1471230
Commit-Queue: Reilly Grant <[email protected]>
Reviewed-by: Yoav Weiss <[email protected]>
Cr-Commit-Position: refs/heads/master@{#695573}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 14, 2019
…ve [NoInterfaceObject], a=testonly

Automatic update from web-platform-tests
[geolocation] Rename interfaces and remove [NoInterfaceObject]

This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f

--

wpt-commits: 99a9f0b73bd0ac3a739dfe2157b93578b760ed5d
wpt-pr: 18989
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Sep 14, 2019
…ve [NoInterfaceObject], a=testonly

Automatic update from web-platform-tests
[geolocation] Rename interfaces and remove [NoInterfaceObject]

This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f

--

wpt-commits: 99a9f0b73bd0ac3a739dfe2157b93578b760ed5d
wpt-pr: 18989
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…ve [NoInterfaceObject], a=testonly

Automatic update from web-platform-tests
[geolocation] Rename interfaces and remove [NoInterfaceObject]

This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f

--

wpt-commits: 99a9f0b73bd0ac3a739dfe2157b93578b760ed5d
wpt-pr: 18989

UltraBlame original commit: d9416fbe9cef4320251f42861d2fd7dbf1bffe18
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…ve [NoInterfaceObject], a=testonly

Automatic update from web-platform-tests
[geolocation] Rename interfaces and remove [NoInterfaceObject]

This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f

--

wpt-commits: 99a9f0b73bd0ac3a739dfe2157b93578b760ed5d
wpt-pr: 18989

UltraBlame original commit: d9416fbe9cef4320251f42861d2fd7dbf1bffe18
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…ve [NoInterfaceObject], a=testonly

Automatic update from web-platform-tests
[geolocation] Rename interfaces and remove [NoInterfaceObject]

This change renames the following interfaces and removes the
[NoInterfaceObject] annotation so that these types are now exposed to
script:

  Coordinates   -> GeolocationCoordinates
  Position      -> GeolocationPosition
  PositionError -> GeolocationPositionError

This is done in response to an effort to remove this annotation from
WebIDL.

Spec pull requests (merged):
w3c/geolocation#20
w3c/geolocation#23

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Xig9oewsQMA/eyC7dbtiAAAJ

Bug: 931847
Change-Id: I38d0172afc33d5757b664e2807356d8727e82d7f

--

wpt-commits: 99a9f0b73bd0ac3a739dfe2157b93578b760ed5d
wpt-pr: 18989

UltraBlame original commit: d9416fbe9cef4320251f42861d2fd7dbf1bffe18
@marcoscaceres
Copy link
Member

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

@anssiko
Copy link
Member

anssiko commented Nov 15, 2019

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

sideshowbarker added a commit to w3c/browser-compat-data that referenced this pull request Jan 30, 2020
w3c/geolocation#23 removed the Navigator*
prefix from the NavigatorGeolocation interface.

w3c/geolocation@26226b5
queengooborg pushed a commit to mdn/browser-compat-data that referenced this pull request Jan 31, 2020
* Deprecate NavigatorGeolocation

w3c/geolocation#23 removed the Navigator*
prefix from the NavigatorGeolocation interface.

w3c/geolocation@26226b5

* Drop NavigatorGeolocation (rather than deprecate)

See #5606 (comment)
@marcoscaceres marcoscaceres mentioned this pull request Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants