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

[bugzilla-daemon at webkit.org]

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





------- Comment #178 from adam at yorba.org  2009-02-16 09:09 PDT -------
(In reply to comment #177)

> > You are misreading the code here. "!error || !*error" written out means...

You're right: I was misreading this.  Thanks for the explanation.  I now agree
that "g_return_val_if_fail (!error || !*error, 0)" makes sense.

I still believe that we don't actually need "if(ec && error)", because
g_set_error_literal() does nothing when passed NULL.  It wouldn't hurt to have
"if(ec && error)" and it might make the code slightly clearer.  On the other
hand, this is generated code which will get duplicated in a zillion places, so
I still might lean toward omitting the "&& error", though I don't care strongly
about this.

> > The easiest way to fix this is to simply stop checking all string arguments for
> > NULL; instead, we can pass them to WebKit and let it complain if it's not happy
> > with a NULL argument.  Can you confirm that this seems like a reasonable idea?
> 
> Letting *WebCore* complain from my experience usually means a segmentation
> fault, I certainly don't want that. What I'm suggesting is to define NULL as
> empty, and return NULL if a value is unset.

I believe that if we were to simply stop checking string attributes for NULL,
you'd have pretty much the behavior you are looking for.  When setting an
attribute of type DOMString, a setter function such as
HTMLTableRowElement::setChOff calls Element::setAttribute(const AtomicString&
name, const AtomicString& value, ExceptionCode& ec) to do its work.  The source
code for that function reveals that if value is NULL, the attribute is removed;
no segmentation fault should result.

Similarly, a getter function such as HTMLTableRowElement::chOff calls
Element::getAttribute(const QualifiedName& name), which returns the null string
if the attribute is not present.  The returned null string is passed to our
converter function gdom_gstring_convert to convert it to a (gchar *); this
function will pass it to g_strdup(), which safely maps NULL to NULL, which will
be returned.

As far as I can tell, in WebKit a string attribute might be either absent, or
the empty string, or a non-empty string.  We could additionally consider
mapping a returned empty string to NULL in an attribute getter; I'm not sure,
but I think you may be saying you want this.  I think I'm uninclined to do
this, since there may be attributes where there is a semantic difference
between NULL and the empty string, and if those should be unified I think it
should probably not happen in this layer.

A more general question is whether we should stop checking all string arguments
for NULL, not merely arguments to attribute setters.  From a quick glance
through the DOM I can't seem to find any methods where passing a NULL string
would make sense.  So I now propose that we stop checking for NULL in string
attribute setters only, and leave the other NULL checks in place.


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