Remove [NoInterfaceObject] #20

anssiko · 2019-02-12T12:24:53Z

Rename exposed interfaces

Fix #19

anssiko · 2019-02-12T12:27:28Z

PTAL @reillyeon

reillyeon

Looks good except for one typo. 😅

reillyeon · 2019-02-12T17:32:30Z

index.html


- callback PositionErrorCallback = void (PositionError positionError);
+ callback PositionErrorCallback = void (PositionErrorPositionError positionError);


s/PositionErrorPositionError/NavigatorGeolocationPositionError/

Amazing typo 🤣

Now fixed.

Rename exposed interfaces Fix w3c#19

anssiko · 2019-02-13T10:20:05Z

@Honry can help update https://github.com/web-platform-tests/wpt/tree/master/geolocation-API accordingly? TL;DR: this patch exposes some interfaces so maybe idlharness.js helps.

Honry · 2019-02-14T01:40:07Z

@foolip's bot will take care of every IDL updates. (i.e. the bot will automatically generate a PR with updated idlharness.js tests to reflect to every IDL updates in all specs.)

anssiko · 2019-02-14T08:36:51Z

Props to @autofoolip for taking care of idlharness.js tests automatically.

IIUC the bot uses IDLs extracted by reffy (here https://github.com/web-platform-tests/wpt/blob/master/interfaces/geolocation-API.idl) that extracts them by crawling the TR version of the spec (in this case https://www.w3.org/TR/geolocation-API). What follows is that the IDL tests will be updated automatically only when a new spec version is published on TR.

My interpretation is we're still compliant with the test as you commit policy we follow in this group. We just need to publish a TR version after any substantial change to the spec IDL to trigger the IDL test update.

@Honry, please correct me if I'm wrong.

Honry · 2019-02-14T08:53:17Z

All refreshed IDLs used by the bot are maintained in https://github.com/tidoust/reffy-reports/tree/master/w3c/idl

Looks like it crawls the ED version of the spec, for example, the bot updated the web-platform-tests/wpt#15125 while the IDL change is not published to TR yet.

anssiko · 2019-02-14T09:06:16Z

@Honry, thanks for the clarification. Using IDL from EDs as the source sounds like a better solution. No issues then.

marcoscaceres

:( late, but this would make a mess in Gecko. It means a ton of refactoring and renaming things/files. As @annevk mentioned, it’s probably not worth doing this at this point. These interfaces are already in browser engines and they don’t cause issues. I’d say switch all these back and just remove NoInterfaceObject only.

marcoscaceres · 2019-08-12T14:55:02Z

Oh wait, did #23 undo this? If yes, yay 😃

marcoscaceres · 2019-08-12T14:56:02Z

Anyway, please please please file bugs on Gecko with changes you are making, otherwise we are all going to get out of sync!

reillyeon · 2019-08-12T14:58:37Z

#23 partially undid the rename. https://chromium-review.googlesource.com/c/chromium/src/+/1471230 is the change to implement this in Blink which I never merged due to some CQ flakiness. I should get on that.

It still renames,

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

marcoscaceres · 2019-08-12T15:50:54Z

Ok, that’s not too bad... but if we can avoid the renaming entirely 🥰

reillyeon · 2019-08-12T16:25:33Z

Wouldn't you agree that the names "coordinates" and "position" are likely to be confused with rendering concepts from other parts of the platform?

marcoscaceres · 2019-08-14T03:16:43Z

Wouldn't you agree that the names "coordinates" and "position" are likely to be confused with rendering concepts from other parts of the platform?

From the engines perspective, no - because they are already implemented specifically for Geolocation.

From an authoring perspective, maybe: They don't have constructors, so they can't be directly used anyway. A new set of coordinates from any new spec could just be named 2DCoordinates and 3DPosition or whatever, without us needing to rename this one (i.e., this name is already squatted).

reillyeon · 2019-08-14T17:13:12Z

If you are firmly opposed to renaming I agree that the names are already squatted and future APIs can work around it. From my perspective this is our last shot before renaming becomes impossible so it seems like a good opportunity to pick something less likely to cause confusion.

anssiko · 2019-08-15T07:00:29Z

@marcoscaceres said:

As @annevk mentioned, it’s probably not worth doing this at this point.

Per my reading of #19 (comment) @annevk is not opposed to the idea of rename, just wanted the group to "carefully consider with what name to expose these interfaces and then please drop this.", where "drop this" refers to [NoInterfaceObject]. I feel the group did carefully consider the names as documented in #19 and #23.

To be clear, the following are the interfaces that dropped [NoInterfaceObject] and the renames to address #19:

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

This seems like a good opportunity for implementers to roll both the (mostly) mechanical rename and the removal of [NoInterfaceObject] into the same patch as in https://chromium-review.googlesource.com/c/chromium/src/+/1471230

Yours,
Anssi the co-chair

marcoscaceres · 2019-08-15T07:25:44Z

I think we need a third opinion here from the WebKit folks - otherwise this exercise is rather pointless. I'm willing to make the change in Gecko if they are also willing to make the change in WebKit. It's a bunch of refactoring work, so I'd personally prefer to spend time on other things. However, if others think this preemptive change is good for the platform, I'm ok to invest time to do the refactoring.

anssiko · 2019-08-15T07:34:33Z

Looping in @cdumez for WebKit feedback. Here's the Blink patch of this spec change:
https://chromium-review.googlesource.com/c/chromium/src/+/1471230

marcoscaceres · 2019-08-15T07:39:53Z

While we are here, can I kindly request that we add:
https://github.com/w3c/screen-orientation/blob/gh-pages/.github/PULL_REQUEST_TEMPLATE.md

That way, we make sure nothing gets merged into the spec unless we have agreement from at least 2 browser vendors, browser bugs are filed, and we have tests?

cdumez · 2019-08-16T15:19:52Z

This looks reasonable. Where is the WebKit bug so that I can work on this?

marcoscaceres · 2019-08-16T16:43:54Z

Curse you, @cdumez 😋 guess I’ll be refactoring too.

anssiko · 2019-08-19T13:31:53Z

@cdumez, the WebKit bug is at https://webkit.org/b/200885 Thanks!

@marcoscaceres, you want me to open one for Gecko? You'll know Mozillians who should be cc'd in the bug, but I'm happy to create one if so preferred.

marcoscaceres · 2019-08-19T13:41:26Z

That would be great! Please cc me and we will go from there.

anssiko · 2019-08-20T06:43:15Z

@marcoscaceres, the Gecko bug is at https://bugzil.la/1575144 Thanks!

marcoscaceres · 2019-08-20T06:44:25Z

Awesome, thanks!

anssiko · 2019-08-26T13:40:08Z

The WebKit bug https://webkit.org/b/200885 is now fixed. That was fast, thanks @cdumez!

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

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 <reillyg@chromium.org> Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Cr-Commit-Position: refs/heads/master@{#695573}

…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

…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 · 2019-11-15T06:51:10Z

Landed in Gecko https://bugzilla.mozilla.org/show_bug.cgi?id=1575144

anssiko mentioned this pull request Feb 12, 2019

Remove [NoInterfaceObject] #19

Closed

reillyeon requested changes Feb 12, 2019

View reviewed changes

Remove [NoInterfaceObject]

923754c

Rename exposed interfaces Fix w3c#19

anssiko force-pushed the remove-nointerfaceobject branch from ee9d941 to 923754c Compare February 13, 2019 10:15

anssiko merged commit de3d4ee into w3c:gh-pages Feb 13, 2019

anssiko deleted the remove-nointerfaceobject branch February 13, 2019 10:22

foolip mentioned this pull request Feb 14, 2019

Rename NavigatorGeolocation* interfaces to Geolocation* #23

Merged

marcoscaceres reviewed Aug 12, 2019

View reviewed changes

anssiko mentioned this pull request Aug 20, 2019

DAS WG TPAC F2F agenda (Fukuoka, Sep 19-20 2019) w3c/devicesensors-wg#24

Closed

chromium-wpt-export-bot mentioned this pull request Sep 11, 2019

[geolocation] Rename interfaces and remove [NoInterfaceObject] web-platform-tests/wpt#18989

Merged

saschanaz mentioned this pull request Nov 18, 2019

Use the new name for Coordinates w3c/webauthn#1344

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove [NoInterfaceObject] #20

Remove [NoInterfaceObject] #20

anssiko commented Feb 12, 2019

anssiko commented Feb 12, 2019

reillyeon left a comment

reillyeon Feb 12, 2019

anssiko Feb 13, 2019

anssiko commented Feb 13, 2019

Honry commented Feb 14, 2019

anssiko commented Feb 14, 2019

Honry commented Feb 14, 2019

anssiko commented Feb 14, 2019

marcoscaceres left a comment

marcoscaceres commented Aug 12, 2019

marcoscaceres commented Aug 12, 2019

reillyeon commented Aug 12, 2019

marcoscaceres commented Aug 12, 2019

reillyeon commented Aug 12, 2019

marcoscaceres commented Aug 14, 2019

reillyeon commented Aug 14, 2019

anssiko commented Aug 15, 2019

marcoscaceres commented Aug 15, 2019

anssiko commented Aug 15, 2019

marcoscaceres commented Aug 15, 2019 •

edited

Loading

cdumez commented Aug 16, 2019

marcoscaceres commented Aug 16, 2019

anssiko commented Aug 19, 2019

marcoscaceres commented Aug 19, 2019

anssiko commented Aug 20, 2019

marcoscaceres commented Aug 20, 2019

anssiko commented Aug 26, 2019

marcoscaceres commented Nov 15, 2019


		callback PositionErrorCallback = void (PositionError positionError);
		callback PositionErrorCallback = void (PositionErrorPositionError positionError);

Remove [NoInterfaceObject] #20

Remove [NoInterfaceObject] #20

Conversation

anssiko commented Feb 12, 2019

anssiko commented Feb 12, 2019

reillyeon left a comment

Choose a reason for hiding this comment

reillyeon Feb 12, 2019

Choose a reason for hiding this comment

anssiko Feb 13, 2019

Choose a reason for hiding this comment

anssiko commented Feb 13, 2019

Honry commented Feb 14, 2019

anssiko commented Feb 14, 2019

Honry commented Feb 14, 2019

anssiko commented Feb 14, 2019

marcoscaceres left a comment

Choose a reason for hiding this comment

marcoscaceres commented Aug 12, 2019

marcoscaceres commented Aug 12, 2019

reillyeon commented Aug 12, 2019

marcoscaceres commented Aug 12, 2019

reillyeon commented Aug 12, 2019

marcoscaceres commented Aug 14, 2019

reillyeon commented Aug 14, 2019

anssiko commented Aug 15, 2019

marcoscaceres commented Aug 15, 2019

anssiko commented Aug 15, 2019

marcoscaceres commented Aug 15, 2019 • edited Loading

cdumez commented Aug 16, 2019

marcoscaceres commented Aug 16, 2019

anssiko commented Aug 19, 2019

marcoscaceres commented Aug 19, 2019

anssiko commented Aug 20, 2019

marcoscaceres commented Aug 20, 2019

anssiko commented Aug 26, 2019

marcoscaceres commented Nov 15, 2019

marcoscaceres commented Aug 15, 2019 •

edited

Loading