Geolocation API LC review comments

Hello Geolocation WG.

Here are some LC review comments from me, not representing the Web Apps
WG.

Thanks,

Cameron



Across the whole document
-------------------------

s/User Agent/user agent/g, since according to the W3C Manual of Style,
it should be written in lowercase.

  http://www.w3.org/2001/06/manual/#Terms



2 Introduction
--------------

  The following code extract illustrates how to obtain basic location
  information:

s/extract/extracts/, since there are multiple examples following.



  // By using a 'timeout' of 0 milliseconds, if there is
  // no suitable cached position available, the User Agent 
  // will immediately invoke the error callback with code
  // TIMEOUT and will not initiate a new position
  // acquisition process.

The comment say that the UA will immediately invoke the error callback,
but the description of getCurrentPosition() in section 5.1 says:

  When called, it must immediately return and then asynchronously
  acquire a new Position object.

So these seem to be in conflict.  It should be made clear in the comment
whether the error callback is invoked before or after
getCurrentPosition() returns.  The same applies to the comment in the
following example.


  navigator.geolocation.getCurrentPosition(successCallback,
                                           errorCallback,
                                           {maximumAge:Infinity, timeout:0});

The type of the maximumAge attribute on the PositionOptions interface is
long.  According to Web IDL, Infinity will be converted to an IDL long
of value 0.

  http://dev.w3.org/2006/webapi/WebIDL/#es-long

If you want to allow Infinity as a value, then you’ll need to either
make maximumAge be of type float or any (and if any, you’d need to
define behaviour when any other type is given).  Alternatively, you
could recommend to pass in the maximum signed 32-bit value to
maximumAge, 2**31-1.



4.3 Additional implementation considerations
--------------------------------------------

  However, in designing these measures, implementers are advised to
  enable user awareness of location sharing, and to provide easy access
  to interfaces that enable revocation of permissions.

s/implementers/implementors/.



5.1 Geolocation interface
-------------------------

  Objects implementing the Navigator interface (e.g. the
  window.navigator object) must also implement the NavigatorGeolocation
  interface [NAVIGATOR].

The current editor’s draft of Web IDL supports asserting this in the IDL
itself with this statement:

  Navigator implements NavigatorGeolocation;

See:

  http://dev.w3.org/2006/webapi/WebIDL/#idl-implements-statements



The indenting of the second block of IDL in 5.1 is a bit strange.  (The
closing brace of the Geolocation interface is not in the first column,
and the following two interfaces aren’t in the first column either.)



I’ll also point out that in the current Web IDL editor’s draft a number
of changes to IDL syntax have been made.  These haven’t been decided on
yet, and probably won’t be until we get to Last Call, but it might be
useful for you to know that optional parameters will probably be
supported using real IDL syntax and not with an [Optional] extended
attribute.  So with the new syntax, the getCurrentPosition() operation
on the Geolocation interface would be rewritten as:

  void getCurrentPosition(in PositionCallback successCallback,
                          in optional PositionErrorCallback errorCallback,
                          in optional PositionOptions options);



It would be good to wrap those operations, since the lines are long and
have white-space:nowrap applied to them currently.



  void clearWatch(in int watchId);

s/int/long/.



  If successful, this method must invoke its associated successCallback
  argument with a Position object as an argument.

I would say s/with a/with that/, since you want to return the object
that was just acquired.

Also, I think you should say that it invokes the handleEvent() operation
of the successCallback object that was passed in, to be language binding
neutral here.  That, or explain somewhere that when it’s written to
invoke one of these callback objects that it means to invoke the
handleEvent() operation on the object.



5.2 PositionOptions interface
-----------------------------

  PositionOptions objects are regular ECMAScript objects that have the
  following properties: 

They are only ECMAScript objects in the ECMAScript binding.  I would
instead say something like:

  In ECMAScript, PositionOptions objects are represented using regular
  native objects with optional properties named "enableHighAccuracy",
  "timeout" and "maximumAge" on them.

and put it below the IDL.



  attribute long timeout;
  attribute long maximumAge;

Should these be unsigned longs instead?  Does it make sense to use
negative values for these attributes?  If not changed, you should define
what happens when negative values are specified for these.



  enableHighAccuracy, timeout and maximumAge attributes are all
  optional: when creating a PositionOptions object, the developer may
  specify any of these attributes.

It doesn’t make sense to say that the attributes (at the IDL level) are
optional.  An object that implements an interface must have those
attributes.  But in ECMAScript, native objects that implement an
interface needn’t have properties to represent those attributes, since
all that happens is that a [[Get]] on the object is performed with the
relevant property name.



  The user might also deny this capability, or the device might not be
  able to provide more accurate results than if the flag wasn't
  specified. 

This implies that omitting the property on an ECMAScript object
corresponds to enableHighAccuracy = false, which certainly is the case
because Web IDL specifies that the undefined value (which is what would
be returned from positionOptionsObject.enableHighAccuracy) converts to
the boolean false, but you may want to point this out explicitly to be
clear.



  If the implementation is unable to successfully acquire a new Position
  before the given timeout elapses, and no other errors have occurred in
  this interval, then the corresponding errorCallback must be invoked
  with a PositionError object whose code attribute is set to TIMEOUT.

Make “code” link to the attribute definition.



Is the default behaviour of converting undefined to 0 appropriate for
the timeout attribute?  You should point out this default behaviour for
when the timeout property isn’t specified on the ECMAScript object.



  Note: Before executing a call to getCurrentPosition() or
  watchPosition(), an implementation may first need to acquire the
  user's permission to disclose location information to the current
  application origin.  In such a case, the timeout interval starts
  immediately after the user's permission has been successfully
  acquired.

This is a note, so it’s non-normative, but it makes requirements of the
timeout interval.  I think this should be normative.



  In case of a getCurrentPosition() call, the errorCallback would be
  invoked exactly once.

s/exactly/at most/, IIUC.



  In case of a watchPosition(), the errorCallback could be invoked
  repeatedly: the first timeout is relative to the moment
  watchPosition() was called, …

I guess it should be relative to the moment watchPosition() was called
or the moment the user’s permission was obtained, if that was necessary.



  If maximumAge is not specified or set to 0, the implementation must
  immediately attempt to acquire a new position object.

At the IDL level, an attribute can’t be not specified.  It’s the
ECMAScript property that can be omitted.  It would be more accurate to
state something like:

  If maximumAge is set to 0, the implementation must immediately attempt
  to acquire a new position object.  Note that the maximumAge attribute
  will default to 0 if an ECMAScript object without a "maximumAge"
  property is used.



  Setting the maximumAge to Infinity will force the implementation to
  return a cached position regardless of its age.

As mentioned above, ∞ isn’t a value of the IDL long type.



5.3 Position interface
----------------------

  This version of the specification allows one attribute of type
  Coordinates and a timestamp.

“timestamp” is underlined in blue for some reason.



  The altitude attribute denotes the height of the position, specified
  in meters above the [WGS84] ellipsoid.  If the implementation cannot
  provide altitude information, the value of this attribute must be
  null.

null isn’t a value of the double type.  If you want to allow null, then
you need to have a type that supports double values or the null value.
In the most recent WD of Web IDL, such a type is defined using the
valuetype syntax:

  valuetype OptionalDouble double;

Then declare the attribute to be of this OptionalDouble type.

See:

  http://www.w3.org/TR/WebIDL/#idl-valuetypes

In recent editor’s drafts, the syntax for this has changed to be a bit
simpler.  There is now a concept of a nullable type (effectively the
same as the boxed valuetypes mentioned above), specified by placing a
“?” after a type name.  So with this new syntax, you would write:

  readonly attribute double? altitude;

But as with the changes to the optionl argument syntax, this hasn’t been
agreed upon yet.

Note that if you wanted to stay within the range of the double type, you
could define the attribute to have a NaN value when the altitude
information is not available.

(These comments apply to altitudeAccuracy, heading and speed, too.)



5.5 PositionError interface
---------------------------

  The specified maximum length of time has elapsed before the
  implementation could successfully acquire a new Position object.

“Position” should be linked to its definition.



6.2 Requirements
----------------

I don’t think the requirements should be section headings with no
content in each section.  Just use an <ol> instead.



References
----------

You should link to dated URLs for W3C specifications, such as Web IDL.
Otherwise, a subsequent publication of the dependent spec could change
incompatibly so that this draft of the Geolocation API effectively
breaks.

-- 
Cameron McCormack ≝ http://mcc.id.au/

Received on Friday, 10 July 2009 02:20:13 UTC