Bug 27437 – make HTMLAppletElement and HTMLEmbedElement compliant with de-facto JS-based standards under gobject bindings

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Misc.

[Luke Kenneth Casson Leighton]

this is part of the series to split #16401 into smaller patches,
as part of an agreement recommended by david.

discussions are awaiting responses from the webkit reviewers on the issues raised by proposing that these two elements comply with de-facto JS-based standards rather than the strict W3C DOM standards.

twelve separate and distinct requests have been made to solicit responses, and all twelve requests have been ignored.  by raising this as a separate butreport, the requests for the discussions to continue which request that the research material be reviewed which show that it is only webkit that complies strictly with the W3C standards on these DOM elements.

as was shown by the XMLHTTPRequest discussions: when these matters come to the attention of other people, my suggestions are actually listened to and discussed reasonably.

i trust that such reasonable discussions will take place on this specific topic.

[Luke Kenneth Casson Leighton]

Created attachment 33066 [details]
patch bringing language gobject bindings up on a par with its peers (de-facto standards compliant) wrt HTMLAppletElement and HTMLEmbedElement

the relevant research material begins here:

https://bugs.webkit.org/show_bug.cgi?id=16401#c121

[Luke Kenneth Casson Leighton]

(In reply to comment #0)
> this is part of the series to split #16401 into smaller patches,
> as part of an agreement recommended by david.
> 
> discussions are awaiting responses from the webkit reviewers on the issues
> raised by proposing that these two elements comply with de-facto JS-based
> standards rather than the strict W3C DOM standards.

 please note: i would not dare to presume to say how the objective-c bindings
 should go, but as the lead developer of a project which, through its users,
 covers absolutely every corner of the DOM bindings, i have a responsibility
 to the users to ensure that the project is useable and useful, and that
 thus extends to its DOM bindings.

 for more insight into the differences between the use of gobject bindings,
 the javascript bindings and the objective-c bindings, see point 2) here:

 https://bugs.webkit.org/show_bug.cgi?id=16401#c218

[Eric Seidel (no email)]

I think these should change to !objc instead.

I don't see why these would only be exposed to JS.

[Mark Rowe (bdash)]

(In reply to comment #3)
> I think these should change to !objc instead.
> 
> I don't see why these would only be exposed to JS.

The members aren't *only* being exposed to JS.  They're being exposed with different types.  Your underlying point is correct though.  The HTML 5 specification marks HTMLEmbedElement's width and height attribute as a DOMString, so in theory all platforms should use the same type for these members.  In practice, Objective-C's DOM is API so they'll need to stay as the different type for sake of API compatibility.

[Mark Rowe (bdash)]

Comment on attachment 33066 [details]
patch bringing language gobject bindings up on a par with its peers (de-facto standards compliant) wrt HTMLAppletElement and HTMLEmbedElement

Per comment 3 and comment 4, the current patch is not correct but the idea is good.  Please revise and resubmit.  Please also include a more useful ChangeLog entry.

[Luke Kenneth Casson Leighton]

(In reply to comment #5)
> (From update of attachment 33066 [details])
> Per comment 3 and comment 4, the current patch is not correct but the idea is
> good.  Please revise and resubmit.  Please also include a more useful ChangeLog
> entry.

 ack will do.

[Luke Kenneth Casson Leighton]

Created attachment 33192 [details]
makes HTMLAppletElement and HTMLEmbedElement width and height DOMString except for Obj-C bindings

is this better?

[Mark Rowe (bdash)]

Comment on attachment 33192 [details]
makes HTMLAppletElement and HTMLEmbedElement width and height DOMString except for Obj-C bindings

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 44473)
> +++ WebCore/ChangeLog	(working copy)
> @@ -62327,6 +62335,21 @@
>          (WebCore::PluginPackage::fetchInfo):
>          (WebCore::PluginPackage::isPluginBlacklisted):
>  
> [phone]  lkcl <[e-mail]>

Please put the correct date and your name here.

> +
> +        Reviewed by NOBODY (OOPS!)
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=27437
> +
> +		Make width and height attributes DOMString except for
> +		LANGUAGE_OBJECTIVE_C due to API backwards-compatibility
> +		reasons.  HTML5 specification has HTMLAppletElement
> +		and HTMLEmbedElement width and height now marked as a
> +		DOMString.

You have a number of tabs here that our pre-commit hook will be unhappy with.

> +
> +        * html/HTMLAppletElement.idl: width and height attributes DOMString except for LANGUAGE_OBJECTIVE_C
> +        * html/HTMLEmbedElement.idl: width and height attributes DOMString except for LANGUAGE_OBJECTIVE_C

It'd be great if these sentences had correct grammar.

Please resubmit with an updated ChangeLog so that this can be landed.

[Luke Kenneth Casson Leighton]

(In reply to comment #8)
> (From update of attachment 33192 [details])
> > Index: WebCore/ChangeLog
> > ===================================================================
> > --- WebCore/ChangeLog	(revision 44473)
> > +++ WebCore/ChangeLog	(working copy)
> > @@ -62327,6 +62335,21 @@
> >          (WebCore::PluginPackage::fetchInfo):
> >          (WebCore::PluginPackage::isPluginBlacklisted):
> >  
> > [phone]  lkcl <[e-mail]>
> 
> Please put the correct date and your name here.

 ack.

> > +
> > +        Reviewed by NOBODY (OOPS!)
> > +
> > +        https://bugs.webkit.org/show_bug.cgi?id=27437
> > +
> > +		Make width and height attributes DOMString except for
> > +		LANGUAGE_OBJECTIVE_C due to API backwards-compatibility
> > +		reasons.  HTML5 specification has HTMLAppletElement
> > +		and HTMLEmbedElement width and height now marked as a
> > +		DOMString.
> 
> You have a number of tabs here that our pre-commit hook will be unhappy with.

 ah, drat.  didn't know that.  did wonder why stuff was on one line.  duh.
 
> > +
> > +        * html/HTMLAppletElement.idl: width and height attributes DOMString except for LANGUAGE_OBJECTIVE_C
> > +        * html/HTMLEmbedElement.idl: width and height attributes DOMString except for LANGUAGE_OBJECTIVE_C
> 
> It'd be great if these sentences had correct grammar.

 *chortle* yes it would.
 
> Please resubmit with an updated ChangeLog so that this can be landed.

 ack.

[Luke Kenneth Casson Leighton]

Created attachment 33328 [details]
corrects grammar, complies with precommit hook, adds correct date and name

[Mark Rowe (bdash)]

Comment on attachment 33328 [details]
corrects grammar, complies with precommit hook, adds correct date and name

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 44473)
> +++ WebCore/ChangeLog	(working copy)
> @@ -62327,6 +62347,17 @@
>          (WebCore::PluginPackage::fetchInfo):
>          (WebCore::PluginPackage::isPluginBlacklisted):
>  
> [phone]  Luke Kenneth Casson Leighton <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!)
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=27437
> +
> +		Make width and height attributes DOMString except for LANGUAGE_OBJECTIVE_C due to API backwards-compatibility reasons.  HTML5 specification has HTMLAppletElement and HTMLEmbedElement width and height now marked as a DOMString.
> +
> +        * html/HTMLAppletElement.idl: width and height attributes made to be DOMString except for LANGUAGE_OBJECTIVE_C where they are kept as long
> +        * html/HTMLEmbedElement.idl: width and height attributes made to be DOMString except for LANGUAGE_OBJECTIVE_C where they are kept as long
> +

It's no longer November 2008 and there are still tabs in your ChangeLog entry…  Can you please fix this minor issue so that this can be landed?

[Luke Kenneth Casson Leighton]

Created attachment 33377 [details]
removes tabs, corrects date

damnit, so sorry, mark - the process of maintaining what is currently 15 separate patches is somewhat cumbersome.  your patience here is really appreciated.  s/^I/    /g got rid of stupid vim-auto-generated tabs.

[Luke Kenneth Casson Leighton]

Comment on attachment 33377 [details]
removes tabs, corrects date

whoops, forgot to flick the review flag...

[Eric Seidel (no email)]

Comment on attachment 33377 [details]
removes tabs, corrects date

Please get Mark Rowe to sign off on this patch.

[Eric Seidel (no email)]

Comment on attachment 33377 [details]
removes tabs, corrects date

Please get Mark Rowe to sign off on this patch.

[Maciej Stachowiak]

Comment on attachment 33377 [details]
removes tabs, corrects date

Luke, everything in this patch looks ok on the substance. The one problem I see is that the ChangeLog hunk is wrong - it has lines above the + lines. This will lead to the patch applying wrong. Please move the ChangeLog entry to the top of WebCore/ChangeLog and resubmit, then I will r+ this. Everything else looks fine. (r- for minor technicality).

[Anders Carlsson]

No point in keeping this bug open.