New issue
Advanced search Search tips

Issue 708206 link

Starred by 2 users
Status: Fixed
Owner:
mcasas@chromium.org
Closed: Apr 2017
Cc:
mcasas@chromium.org
foolip@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 674593


Show other hotlists

Hotlists containing this issue:
Geolocation
Non-Standard-IDL


Sign in to add a comment

Rename the Geoposition interface to just Position

Project Member Reported by foolip@chromium.org, Apr 4 2017 
https://www.w3.org/TR/geolocation-API/#api_description

The interface has [NoInterfaceObject], so this is unlikely to have any impact, but can be observed by calling instance.toString() on an instance.

By matching the spec's name for things, tooling will be better able to find any differences between the members, now or in the future.

Comment 1 by mcasas@chromium.org, Apr 4 2017

Cc: mcasas@chromium.org
Labels: Hotlist-GoodFirstBug

Comment 2 by juncai@chromium.org, Apr 5 2017

Status: Available (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80d6493c57a368cf14c8c3eaf429732250df0e9e

commit 80d6493c57a368cf14c8c3eaf429732250df0e9e
Author: mcasas <mcasas@chromium.org>
Date: Tue Apr 11 20:47:57 2017

Geolocation: rename Geoposition to Position to follow Spec

Renamed Geoposition to Position to align with the spec [1] and
rippled that change through the files.  This change does not
surface to the Web platform since Geoposition is a
NoInterfaceObject _except_ for the way Geoposition is
stringified, which changes to "[object Position]".

[1] https://www.w3.org/TR/geolocation-API/#position_interface

BUG= 708206 

Review-Url: https://codereview.chromium.org/2809843002
Cr-Commit-Position: refs/heads/master@{#463759}

[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/LayoutTests/geolocation-api/position-string-expected.txt
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/LayoutTests/geolocation-api/position-string.html
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/BUILD.gn
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/GeoNotifier.cpp
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/GeoNotifier.h
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/Geolocation.h
[rename] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/Position.h
[rename] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/Position.idl
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/PositionCallback.h
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/geolocation/PositionCallback.idl
[modify] https://crrev.com/80d6493c57a368cf14c8c3eaf429732250df0e9e/third_party/WebKit/Source/modules/modules_idl_files.gni

Comment 4 by mcasas@chromium.org, Apr 11 2017

Cc: foolip@chromium.org
Owner: mcasas@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/974b5ce01edf7431419fd59151581fcaa8397dfd

commit 974b5ce01edf7431419fd59151581fcaa8397dfd
Author: mcasas <mcasas@chromium.org>
Date: Tue Apr 11 22:13:19 2017

Revert of Geolocation: rename Geoposition to Position to follow Spec (patchset #4 id:80001 of https://codereview.chromium.org/2809843002/ )

Reason for revert:
Seems like Position was already taken (by
core\editing\position.h), reverting due to comment
#29

I'm puzzled that this didn't trigger _any_ bot :?

Original issue's description:
> Geolocation: rename Geoposition to Position to follow Spec
>
> Renamed Geoposition to Position to align with the spec [1] and
> rippled that change through the files.  This change does not
> surface to the Web platform since Geoposition is a
> NoInterfaceObject _except_ for the way Geoposition is
> stringified, which changes to "[object Position]".
>
> [1] https://www.w3.org/TR/geolocation-API/#position_interface
>
> BUG= 708206 
>
> Review-Url: https://codereview.chromium.org/2809843002
> Cr-Commit-Position: refs/heads/master@{#463759}
> Committed: https://chromium.googlesource.com/chromium/src/+/80d6493c57a368cf14c8c3eaf429732250df0e9e

TBR=foolip@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 708206 

Review-Url: https://codereview.chromium.org/2809783004
Cr-Commit-Position: refs/heads/master@{#463797}

[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/LayoutTests/geolocation-api/position-string-expected.txt
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/LayoutTests/geolocation-api/position-string.html
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/BUILD.gn
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/GeoNotifier.cpp
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/GeoNotifier.h
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/Geolocation.h
[rename] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/Geoposition.h
[rename] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/Geoposition.idl
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/PositionCallback.h
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/geolocation/PositionCallback.idl
[modify] https://crrev.com/974b5ce01edf7431419fd59151581fcaa8397dfd/third_party/WebKit/Source/modules/modules_idl_files.gni

Comment 6 by mcasas@chromium.org, Apr 11 2017

Status: Assigned (was: Fixed)

Comment 7 by mcasas@chromium.org, Apr 12 2017

Status: Started (was: Assigned)
Position is used in [1]:
using Position = PositionTemplate<EditingStrategy>;

I'll reland and make sure this using is correctly contained.


[1] https://cs.chromium.org/webrtc/src/third_party/WebKit/Source/core/editing/Position.h?q=core/editing/position.h&dr&l=224
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 13 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8

commit c73d5721559290e45c7d3aa8b5d1a5ef6caaade8
Author: mcasas <mcasas@chromium.org>
Date: Thu Apr 13 06:26:30 2017

Geolocation: rename Geoposition to Position to follow Spec (take 2)

CL https://crrev.com/2809843002 took  a stab at it but clashed with
the definition in [1]:
   using Position = PositionTemplate<EditingStrategy>;
which is, sadly, used in a bazillion places (see e.g. [2]).

This CL takes a least bad solution by renaming the idl-exposed
Geoposition to Position, but keeping the underlying c++ classes
unchanged.

BUG= 708206 

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/Position.h?dr&sq=package:chromium&l=224
[2] https://cs.chromium.org/search/?q=%22+Position+%22+file:%5Esrc/third_party/WebKit/Source/core/+package:%5Ewebrtc$&type=cs

Review-Url: https://codereview.chromium.org/2816713004
Cr-Commit-Position: refs/heads/master@{#464321}

[modify] https://crrev.com/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8/third_party/WebKit/LayoutTests/geolocation-api/position-string-expected.txt
[modify] https://crrev.com/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8/third_party/WebKit/LayoutTests/geolocation-api/position-string.html
[rename] https://crrev.com/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8/third_party/WebKit/Source/modules/geolocation/Position.idl
[modify] https://crrev.com/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8/third_party/WebKit/Source/modules/geolocation/PositionCallback.idl
[modify] https://crrev.com/c73d5721559290e45c7d3aa8b5d1a5ef6caaade8/third_party/WebKit/Source/modules/modules_idl_files.gni

Comment 10 by mcasas@chromium.org, Apr 13 2017

Status: Fixed (was: Started)

Comment 11 by benhenry@chromium.org, Sep 22 2017

Components: Blink>Geolocation

Comment 12 by benhenry@chromium.org, Sep 22 2017

Components: -Blink>Location
►

Sign in to add a comment