Re: draft-ietf-httpbis-client-hints-00 feedback

Hi Julian.

On Tue, Mar 29, 2016 at 11:38 AM, Julian Reschke <julian.reschke@gmx.de>
wrote:
>
> (I'm happy to provide pull requests for the nits...)
>

And I'm happy to accept them! :-)

I've started a branch with a bunch of fixes, please feel free to update
that or make a pull against it:
https://github.com/httpwg/http-extensions/pull/163

Once we're happy, we can squash and land it.


> Abstract
>
>    An increasing diversity of Web-connected devices and software
>    capabilities has created a need to deliver optimized content for each
>    device.
>
>    This specification defines a set of HTTP request header fields,
>    colloquially known as Client Hints, to address this.  They are
>    intended to be used as input to proactive content negotiation; just
>    as the Accept header allows clients to indicate what formats they
>
> NIT: s/header/header field/ (multiple times in document)
>

Ack.


>    prefer, Client Hints allow clients to indicate a list of device and
>    agent specific preferences.
>
> The spec should be clear whether it's about *clients* or *user agents* (I
> believe it is about user agents, right?)
>

I don't think we want to restrict it to "user agents", even if that's the
primary and motivating use case. For examples, apps are another consumer. I
guess we should clarify that in the intro, which is currently very
UA-focused.



> 2.  Client Hint Request Header Fields
>
>    This document defines a selection of Client Hint request header
>    fields, and can be referenced by other specifications wishing to use
>    the same syntax and processing model.
>
> NIT: true, but there doesn't seem to be a common syntax after all. A
> leftover from earlier versions?
>

Ack.


>
> 2.1.  Sending Client Hints
>
>    Clients control which Client Hint headers and their respective header
>    fields are communicated, based on their default settings, user
>
> That really doesn't parse...
>

I think we'll address this via:
https://github.com/httpwg/http-extensions/issues/156



>    configuration and/or preferences.  The user may be given the choice
>    to enable, disable, or override specific hints.
>
> Please avoid lowercase RFC2119 terms. In this case, "can" seems to be
> right.


>    The client and server, or an intermediate proxy, may use an opt-in
>    mechanism to negotiate which fields should be reported to allow for
>    efficient content adaption.
>
> Same here.
>

Ack.

2.2.  Server Processing of Client Hints
>
>    Servers MAY respond with an optimized response based on one or more
>    received hints from the client.  When doing so, and if the resource
>
> No need for a MAY here. It's a statement of fact.
>

Ack.


>
>    Further, depending on the used hint, the server MAY also need to emit
>    additional response header fields to confirm the property of the
>
> ...can...
>

Ack.


>    response, such that the client can adjust its processing.  For
>    example, this specification defines "Content-DPR" response header
>    field that MUST be returned by the server when the "DPR" hint is used
>    to select the response.
>
> Please have the normative requirement just once (down where DPR is
> defined).
>

Ack.


>    When a client receives Accept-CH, it SHOULD append the Client Hint
>    headers that match the advertised field-values.  For example, based
>    on Accept-CH example above, the client would append DPR, Width,
>    Viewport-Width, and Downlink headers to all subsequent requests.
>
> Not sure whether really is a SHOULD. Is a client that chooses to implement
> only one of the hints in violation of the spec?
>

No, we should fix that as part of
https://github.com/httpwg/http-extensions/issues/156.


> 2.2.2.  Interaction with Caches
>
>    When selecting an optimized response based on one or more Client
>    Hints, and if the resource is cacheable, the server MUST also emit a
>    Vary response header field ([RFC7234]) to indicate which hints were
>    used and whether the selected response is appropriate for a later
>    request.
>
> (see above)
>

Ack.


>
>      Vary: DPR
>
>    Above example indicates that the cache key should be based on the DPR
>    header.
>
> s/should be based/needs to include/
>

Ack.


>    Client Hints MAY be combined with Key ([I-D.ietf-httpbis-key]) to
>    enable fine-grained control of the cache key for improved cache
>    efficiency.  For example, the server MAY return the following set of
>    instructions:
>
> s/MAY/can/
>

Ack.


> 3.  The DPR Client Hint
>
>    The "DPR" header field is a number that, in requests, indicates the
>    client's current Device Pixel Ratio (DPR), which is the ratio of
>    physical pixels over CSS px of the layout viewport on the device.
>
> What does it mean in responses then? It might be simpler to say "request
> header field" (this applies to most of the definitions)
>

Ack.


> Also include a reference to "CSS px".
>
>      DPR = 1*DIGIT [ "." 1*DIGIT ]
>
>    If DPR occurs in a message more than once, the last value overrides
>    all previous occurrences.
>
> It might be good to state that it's invalid to include it multiple times
> (applies to most definitions).
>

Any suggestions on language for this?


>      Content-DPR = 1*DIGIT [ "." 1*DIGIT ]
>
>    DPR ratio affects the calculation of intrinsic size of image
>    resources on the client - i.e. typically, the client automatically
>    scales the natural size of the image by the DPR ratio to derive its
>    display dimensions.  As a result, the server must explicitly indicate
>    the DPR of the selected image response whenever the DPR hint is used,
>    and the client must use the DPR value returned by the server to
>    perform its calculations.  In case the server returned Content-DPR
>
> These are MUSTs, right?
>
>    value contradicts previous client-side DPR indication, the server
>    returned value must take precedence.
>
> Same here...
>

Ack.


>    Note that DPR confirmation is only required for image responses, and
>    the server does not need to confirm the resource width as this value
>    can be derived from the resource itself once it is decoded by the
>    client.
>
> As we have a normative requirement here, we probably need to state what an
> "image resource" exactly is.
>

Image content-type? Do we have this defined anywhere?


> 4.  The Width Client Hint
>
>    The "Width" header field is a number that, in requests, indicates the
>    resource width in physical px (i.e. intrinsic size of an image).  The
>    provided physical px value is a number rounded to the largest
>    smallest following integer (i.e. ceiling value).
>
>
>      Width = 1*DIGIT
>
>    If the resource width is not known at the time of the request or the
>    resource does not have a display width, the Width header field may be
>    omitted.  If Width occurs in a message more than once, the last value
>    overrides all previous occurrences.
>
> It took me some time to understand that this is the width the user agent
> intends to use to display the resource. Maybe this could be rephrased.
>

Hmm, no. Physical width != display width. For example, on a "2x" screen a
100 (CSS) px resource has an intrinsic size of 200px. FWIW, this language
follows CSS spec, and I'd prefer to keep it aligned.


>
> 8.  Examples
>
>    For example, given the following request headers:
>
>
>      DPR: 2.0
>      Width: 320
>      Viewport-Width: 320
>
>    The server knows that the device pixel ratio is 2.0, that the
>    intended display width of requested resource is 160 CSS px (320
>
> s/of/of the/
>

Ack.


> 9.  Security Considerations
>
>    Client Hints defined in this specification do not expose any new
>    information about the user's environment beyond what is already
>    available to, and may be communicated by, the application at runtime
>    via JavaScript - e.g. viewport and image display width, device pixel
>    ratio, and so on.
>
>    However, implementors should consider the privacy implications of
>    various methods to enable delivery of Client Hints - see "Sending
>    Client Hints" section.  For example, sending Client Hints on all
>
> Section #...
>
>    requests may make information about the user's environment available
>    to origins that otherwise did not have access to this data (e.g.
>    origins hosting non-script resources), which may or not be the
>    desired outcome.  The implementors may want to provide mechanisms to
>    control such behavior via explicit opt-in, or other mechanisms.
>    Similarly, the implementors should consider how and whether delivery
>    of Client Hints is affected when the user is in "incognito" or
>    similar privacy mode.
>
> (may, should, etc...)
>

Ack.


>
> 10.  IANA Considerations
>
>    o  Header field name: DPR
>    o  Applicable protocol: HTTP
>    o  Status: standard
>    o  Author/Change controller: IETF
>    o  Specification document(s): [this document]
>
> ...insert section # (applies to all definitions)
>

Hmm, does our tooling allow us to auto-generate these? =/


> 11.  Normative References
>
>    [I-D.ietf-httpbis-key]
>               Fielding, R. and m. mnot, "The Key HTTP Response Header
>               Field", draft-ietf-httpbis-key-00 (work in progress),
>               October 2015.
>
> ...the autogenerated ref is broken :-)
>

@mnot: what should that be?

---

Phew. Thanks for the thorough review!

Received on Wednesday, 30 March 2016 02:39:02 UTC