Bug 27603 – updating Document.idl to match HTML5 spec: domain read-write and access to getSelection

• Component: WebCore Misc.
• Status: UNCONFIRMED

[Luke Kenneth Casson Leighton]

carrying on from discussion in https://bugs.webkit.org/show_bug.cgi?id=27434 comment https://bugs.webkit.org/show_bug.cgi?id=27434#c3 suggests that the domain property be made read-write and that access to getSelection be made universal to all bindings, to match the HTML5 specification.

[Luke Kenneth Casson Leighton]

Created attachment 33323 [details]
makes Document.domain read-write and adds Document.getSelection to all bindings, as per HTML5 spec

[Mark Rowe (bdash)]

Comment on attachment 33323 [details]
makes Document.domain read-write and adds Document.getSelection to all bindings, as per HTML5 spec

>  #if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
>                   attribute [ConvertNullToNullString] DOMString domain;
>  #else
> -        readonly attribute DOMString domain;
> +        attribute DOMString domain;
>  #endif

It doesn't seem necessary to have this be #if'd any more.

> +        // now in HTML5 specification
> +        DOMSelection       getSelection();

The comment doesn't add anything.

> [phone]  lkcl <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!)
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=27603
> +
> +		https://bugs.webkit.org/show_bug.cgi?id=27434#c3 suggests that the
> +		domain property be made read-write and that access to getSelection be
> +		made universal to all bindings, to match the HTML5 specification.
> +
> +        * dom/Document.idl: updated to match HTML spec on domain and getSelection

Your ChangeLog entry again contains tabs that will make our pre-commit hook unhappy.

[Luke Kenneth Casson Leighton]

(In reply to comment #2)
> (From update of attachment 33323 [details])
> >  #if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
> >                   attribute [ConvertNullToNullString] DOMString domain;
> >  #else
> > -        readonly attribute DOMString domain;
> > +        attribute DOMString domain;
> >  #endif
> 
> It doesn't seem necessary to have this be #if'd any more.

 ah... oh?  oh, what, because the [ConvertNullToNullString] gets ignored by bindings generators that don't need it?  oh, ok.  great.  less code!

> > +        // now in HTML5 specification
> > +        DOMSelection       getSelection();
> 
> The comment doesn't add anything.

 oh, ok.  just trying to be helpful.

> > [phone]  lkcl <[e-mail]>
> > +
> > +        Reviewed by NOBODY (OOPS!)
> > +
> > +        https://bugs.webkit.org/show_bug.cgi?id=27603
> > +
> > +		https://bugs.webkit.org/show_bug.cgi?id=27434#c3 suggests that the
> > +		domain property be made read-write and that access to getSelection be
> > +		made universal to all bindings, to match the HTML5 specification.
> > +
> > +        * dom/Document.idl: updated to match HTML spec on domain and getSelection
> 
> Your ChangeLog entry again contains tabs that will make our pre-commit hook
> unhappy.

 achhh, yes, that's vi messing up (ok - me messing up using vi) sorry.  now i know, i've done s/^I/    /g but there may be a couple of entries still with tabs, i'll try to find them.

[Luke Kenneth Casson Leighton]

> > +        // now in HTML5 specification
> > +        DOMSelection       getSelection();
> 
> The comment doesn't add anything.

 ahh, yes it does.  look carefully at the original file, and you
 can see that getSelection() _used_ to be commented
 as a "Mozilla Extension".

 if you remove only the #ifdef, and don't provide a comment,
 it looks like getSelection() is an IE Extension (not an
 HTML5ism) because that's the commented-block above it.


        // IE extensions

                 attribute [ConvertNullStringTo=Undefined, ConvertNullToNullString] DOMString charset;
        readonly attribute [ConvertNullStringTo=Undefined] DOMString defaultCharset;
        readonly attribute [ConvertNullStringTo=Undefined] DOMString readyState;

        Element            elementFromPoint(in long x, in long y);

        // HTML 5
        DOMSelection       getSelection();

        // Mozilla extensions

        readonly attribute [ConvertNullStringTo=Null] DOMString characterSet;


 looking _further_ down the Document.idl file, you can see
 that there is in fact a comment "// HTML 5" already in there:

        // HTML 5
        NodeList getElementsByClassName(in DOMString tagname);


 so, anticipating that "HTML 5" is acceptable, because it's already
 _in_ the file, i'm resubmitting a patch which contains the exact
 same comment "HTML 5".

 the alternative is to move the function getSelection() to somewhere
 which would only confuse people.

[Luke Kenneth Casson Leighton]

Created attachment 33384 [details]
removes JS-specificism around Document.domain; keeps comment on getSelection to avoid it being confused with IE block of properties, above, but makes comment same as is shown further down in file

[Eric Seidel (no email)]

Comment on attachment 33384 [details]
removes JS-specificism around Document.domain; keeps comment on getSelection to avoid it being confused with IE block of properties, above, but makes comment same as is shown further down in file

I believe getSelection is intentionally avoided for Obj-C bindings, but I"m not sure.  Please have an Apple person, like Mark Rowe or Tim Hatcher comment as to if this change is correct or not.

[Eric Seidel (no email)]

Comment on attachment 33384 [details]
removes JS-specificism around Document.domain; keeps comment on getSelection to avoid it being confused with IE block of properties, above, but makes comment same as is shown further down in file

Removing my review.