Re: Publishing a new version of the WD

Hi,

I've now changed jobs and been added back to the group as a representative
of Facebook. To reply belatedly (inline)

On Thu, Nov 8, 2012 at 9:42 AM, Ross Patterson <rpatterson@parature.com>wrote:

> I'm not a member of the group, so this isn't an objection.  But I have a
> few comments from reading the spec without the benefit of hearing it worked
> out, and I think that's an important perspective
>
>
> * General:
>         * There are at least two styles of description for protocol
> interactions.  The IDL style used in sec. 3.2.1 seems to me to be more
> rigorous than the table style in sec. 5.2.
>

Yes. I'm aware of this. I think we're going to move to a style that's
closer to HTML5's: the DL approach we take tends to lead to abbreviated
content as we explain the contents. That probably won't be complete by the
time we publish the WD, but it should be improved.


>         * I think capability descriptions might be much clearer if they
> were centralized into sec. 3 instead of being scattered throughout the
> document.  For example, the "secureSsl" capability is described in section
> 5.2.1 as if it were peculiar to the "get" command, but it appears to be
> related to any page transition that might occur, not just a transition
> initiated by this command.
>

I plan on adding an appendix collecting these all together. However, the
capabilities reflect the fact that a browser supports a particular feature,
so it makes the most sense to keep the capability name in the section that
describes what it provides. As such, the appendix will be non-normative.


> * Sec. 2.4:
>         * SessionNotCreatedException (sec. 4.1) is not defined.
>         * NoSuchWindow (sec. 6.4 et al.) is not defined.
>         * StaleElementReferenceException (sec. 11.2 et al.) is not defined
>

Good catch. I've already sorted out some of these in earlier revisions of
the doc, but it's clear that there's more work to do.


> * Sec. 4.1:
>         * Step 5 says "If the Command object had the "sessionId" field
> set, this may be discarded in favour of the freshly generated sessionId."
>  I believe "... this may be discarded ..." should be read as "... the
> remote end may ignore the Command's sessionId ...".  It might also be
> worthwhile to state that the local end MUST use the sessionId returned in
> the Response, even if it differs from the Command.
>

Excellent point. I'll reword that section.


>         * The SessionNotCreatedException descriptions speak of "the error
> message" and "the value", which I believe should be read as "a string
> returned as the value attribute of the Response".
>

Agreed.


>         * Experience with other protocols (e.g., SMTP, HTTP) has shown
> that it can be useful for the local end to know what capabilities the
> remote end offers.  If the Capabilities described in step 6 were allowed to
> contain additional values supplied by the remote end, this would be
> possible.  One example of its use might be a Boolean "commandPipelining"
> capability, indicating the remote end's willingness to accept and queue
> commands while other commands are in progress, as discussed in sec. 5.3.
>

The remote end is free to send additional capabilities in existing
implementations. I should spell this out in the spec.


> * Sec. 4.1.1 says "vendor code (eg: "MOZ-B2G")."  Shouldn't these be "-" +
> vendorPrefix + "-" + value and not upper-cased, like the extensions
> described in section 18.1?
>

Yes :)


> * Sec 4.1.2 is missing the SessionNotCreated status code description.
>

I'll add one before we publish the WD.


> * Sec. 5.1:
>         * The spec should use words (e.g., "or", "equal") instead of
> operators derived from an unspecified language (e.g., "==", "||").  While
> most programmers will place ISO-C or Java interpretations upon operators,
> there are other interpretations as well.
>

I'd not realised that this could be ambiguous. In this case words would be
clearer.


> * Sec. 5.2.1:
>         * This section describes the secureSsl capability, which defaults
> to the opposite of normal browser operation (i.e., false instead of true).
>  Is the "sorry fact" mentioned in the NOTE truly so common that this
> capability should have such an unusual default?
>

Absolutely. I've only ever seen one company that uses HTTPS on its test
environments that's configured correctly. Until we set this as the default
in the OSS webdriver project we used to get regular queries about how to
handle it.


>         * Is the MAY in "... implementations MAY choose to make accessing
> a site with bad HTTPS ..." really justified?  If the local end has
> explicitly overridden the default of secureSsl=false, shouldn't this really
> be a MUST?  Especially if it was provided in a requiredCapabilities?
>

If it's in a requiredCapabilities, the session should not have started. If
it's a desiredCapability then the browser can safely ignore it. The reason
why I used MAY is that some existing implementations don't support this
feature yet (notably the IE driver) and I can imagine some browser vendors
wanting to take a "pure" approach to handling SSL even if it makes testing
harder. Personally, I think that's a terrible idea, but there's been debate
about it in the OSS project.


> * Sec. 6.3 says "The ordering of the keys is not defined ...", which I
> think should be read as "The ordering of the array elements is not defined
> ...".
>

Agreed.


> * Sec. 6.4 does not define the effect of the "fullscreenWindow" command.
>

We've not written that bit of the spec yet. I'll add at least a place
holder before we go to WD.


> * Sec. 7.2 conflates three different parameters as "name".  Shouldn't
> there instead be "id", "windowHandle", and "windowName" parameters with
> separate value sets?
>

The "name" parameter could mean one of several things. That list describes
the order in which that parameter should be compared with attributes of the
window. The reason for doing things this way is to make the local end
easier to implement: each of these types is commonly represented as a
string. Given we're not even strongly typing "windowHandle" the local end
(it's an opaque string) a poorly designed framework layered on top of
WebDriver will cause all sorts of trouble. This way, we push the complexity
into the remote end but can clearly express behaviour.


> * Sec. 7.3 conflates four different parameters as "id".  Shouldn't there
> instead be "frameNumber", "frameId", "name", and id parameters with
> separate value sets?
>

As above, but with an additional parameter.


> * Sec. 9 discusses the difference between a WebDriver "id" attribute and a
> DOM "id" attribute.  Given the huge history of ignoring the uniqueness
> requirement for DOM's "id", the "The IDs used to refer to different
> underlying DOM Elements must be unique within the session over the entire
> duration of the session." Should probably explicitly reject the historical
> illegal usage of non-unique DOM "id" values.
>

Isn't that expressed in "must be unique"? Or does it need to be made even
clearer?


> * Sec. 9.1's NOTE does not appear to be related to the section's content.
>

That's a mistake. It should be associated with the opening paragraph.


> * Sec. 10.2 references a "getProperty" method that isn't defined anywhere
> in the spec.
>

I wonder where that went. Could have sworn it was in there at one point.


> * Sec. 12 and other later sections speak of "remote implementations".  I
> think this should be read as "remote ends".
>
> * Sec. 16 and other later sections speak of "local implementations".  I
> think this should be read as "local ends".
>

Agreed on both points.


> * Sec. 16.2 says "One of those monitors would be very cool."  Very true,
> but also the only instance of humor in the spec so far :-)
>

The second: section 1.1 has The Other Joke in it. :)

Once again. Thank you for the feedback. I really appreciate it.

Regards,

Simon


>
> Ross Patterson
> Parature
>
> -----Original Message-----
> From: Simon Stewart [mailto:simon.m.stewart@gmail.com]
> Sent: Tuesday, October 30, 2012 7:22 AM
> To: public-browser-tools-testing
> Subject: Publishing a new version of the WD
>
> Hi,
>
> The members of this group who were at TPAC voted on whether to publish a
> new working draft of the WebDriver spec. The vote was unanimously in
> favour. The current version can be seen at:
>
> http://dvcs.w3.org/hg/webdriver/raw-file/tip/webdriver-spec.html
>
> Before publishing that spec, are there any objections from members of this
> group? I shall wait until Friday before moving forward with the publishing
> process to allow time for objections. If any are raised, we'll attempt to
> deal with them before publishing.
>
> Regards,
>
> Simon
>
>
>