[Webkit-unassigned] [Bug 16401] [GTK] GObject/C DOM binding

[bugzilla-daemon at webkit.org]

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





--- Comment #234 from Luke Kenneth Casson Leighton <lkcl at lkcl.net>  2009-07-15 12:28:36 PDT ---
(In reply to comment #215)
> > After re-reviewing some of the comments, I think my comment that the patch
> > "must be broken up in to smaller pieces" may be worded too strongly.  In
> > Comment #76, breaking up the patch is suggested as a best practice for new
> > contributors and will lead to quicker turnaround (reviews) on posted patches,
> > but it's not a strict requirement.
> 
>  ah, whew, that's a relief.
> 
> > However, since you're unwilling to break it up
> 
>  it's more than that, it's that by doing so i cannot make any guarantees that
> the individual portions are either correct or actually do anything.  it was
> only by having pretty much absolutely everything that i could get
> pyjamas-desktop to work, and thus run the high-level tests (pyjamas examples,
> pyjamas unit tests etc.)
> 
> > (and since willing to be patient),
> 
>  got all the time in the world.  the main reason for that is because i managed
> at europython to port pyjamas-desktop to the competitor to pywebkitgtk:
> python-xpdom.  it uses XUL (mozilla).  so, pyjamas developers can develop
> applications to run in web browsers (compiled to javascript) _or_ they can run
> them as desktop apps...
> 
> .... no longer using webkit, but using mozilla technology instead.
> 
> > the next step would be to post a new monolithic patch for review
> > addressing feedback in Comment #191 and Comment #194.
> 
>  ok.  the comments are related to a (highly modified) patch which i could not
> get involved in, because of the removal of the event handling.
> 
>  so, *sigh* it's necessary to take two steps back in order to take one step
> forward, for which i apologise (both to you, and to adam - sorry, adam, but
> removing the event handling, i know why you did it, as it took me three days to
> update to the latest svn, last week, but i have to work from what i know and
> what i can prove works).
> 
>  so i'll endeavour to address the comments where they are relevant.
> 
>  one immediately that can be addressed: custom attributes.
> 
> > > +    if ($attribute->signature->extendedAttributes->{"Custom"} ||
> > > +    	$attribute->signature->extendedAttributes->{"CustomGetter"} ||
> > > +        $attribute->signature->extendedAttributes->{"CustomSetter"}) {
> > > +        return 1;
> > > +    }
> 
> > [Custom] and related attributes are only used for implementing JS specific
> > quirks and should not be needed for GObject bindings. 
> 
>  the assertion that they are JS-relevant-only is not true, and you can
>  test this by removing the "custom" test in the generator, and trying to
>  add everything.
> 
>  i found this out myself by accidentally removing it, thanks to an
>  error in the generator logic, whoops :)

 so - to demonstrate this to you, sam, i'm trying out the removal of the code
from the generator that tests for custom attributes.

and immediately you get this:

DerivedSources/gdom/GdomDOMWindow.cpp:625: error: ‘class WebCore::DOMWindow’
has no member named ‘event’

WebCore::DOMWindow() _has_ no function "event()", which is correct!

because.... it's a custom function!

and, if we were to implement that custom function, it would only _increase_
the size of the patch, rather than decrease it.  which is contrary to the
wishes of the webkit team [and common sense].

so, this should serve to demonstrate to you that there are sound
technical and practical reasons for doing what is being done.

you are welcome to question each and every single one of the technical
decisions: i can tell you right now that we will be here for a loooong
time.

or, we can land the patch and then retro-grade fix any issues, which
i am still willing to help do.

the choice i leave up to you.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.