Bug 42115 - [GTK] Improper webkit_dom_document_evaluate implementation.
: [GTK] Improper webkit_dom_document_evaluate implementation.
Status: RESOLVED FIXED
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-07-12 15:16 PST by
Modified: 2010-08-28 13:56 PST (History)


Attachments
Patch that solves bug #42115 (2.55 KB, patch)
2010-08-28 06:53 PST, mlq@pwmt.org
xan.lopez: review-
Review Patch | Details | Formatted Diff | Diff
Updated patch (Added Changelog entry) (3.34 KB, patch)
2010-08-28 07:24 PST, mlq@pwmt.org
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (3.17 KB, text/plain)
2010-08-28 07:33 PST, mlq@pwmt.org
no flags Details
Updated (Changed Changelog, ..) (3.17 KB, patch)
2010-08-28 08:38 PST, mlq@pwmt.org
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (Genereated with git format-patch) (3.54 KB, patch)
2010-08-28 09:37 PST, mlq@pwmt.org
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-07-12 15:16:11 PST
Hey,

I just tried to evaluate a simple XPathExpression while I came across the
webkit_dom_document_evaluate() function which seems not to be implemented
correctly.

Like
http://www.w3.org/TR/DOM-Level-3-XPath/xpath.html#XPathEvaluator-evaluate
says the in_result parameter maybe NULL but the assert prohibits a NULL
value.
Even if I try to pass an WebKitDOMXPathResult Object the function segfaults
randomly.

I hope I made myself clear.

Best regards,
Oliver
------- Comment #1 From 2010-07-12 15:28:17 PST -------
A little snippet how I pass an XPathResult object:


WebKitDOMXPathNSResolver *ns = webkit_dom_document_create_ns_resolver(dom, body);
WebKitDOMXPathResult *in = g_object_new(WEBKIT_TYPE_DOM_XPATH_RESULT, NULL);
WebKitDOMXPathResult *result = webkit_dom_document_evaluate(dom, "//a | //button | //textarea | //select | //input[not(@type='hidden')]", WEBKIT_DOM_NODE(dom), ns, 7, in, NULL);

and then it goes down


** (jumanji-debug:20791): CRITICAL **: WebCore::XPathResult* WebKit::core(WebKitDOMXPathResult*): assertion `coreObject' failed

** (jumanji-debug:20791): CRITICAL **: WebKitDOMXPathResult* webkit_dom_document_evaluate(WebKitDOMDocument*, gchar*, WebKitDOMNode*, WebKitDOMXPathNSResolver*, gushort, WebKitDOMXPathResult*, GError**): assertion `converted_in_result' failed
------- Comment #2 From 2010-07-14 04:39:30 PST -------
(In reply to comment #1)
> A little snippet how I pass an XPathResult object:
> 
> 
> WebKitDOMXPathNSResolver *ns = webkit_dom_document_create_ns_resolver(dom, body);
> WebKitDOMXPathResult *in = g_object_new(WEBKIT_TYPE_DOM_XPATH_RESULT, NULL);
> WebKitDOMXPathResult *result = webkit_dom_document_evaluate(dom, "//a | //button | //textarea | //select | //input[not(@type='hidden')]", WEBKIT_DOM_NODE(dom), ns, 7, in, NULL);
> 
> and then it goes down
> 
> 
> ** (jumanji-debug:20791): CRITICAL **: WebCore::XPathResult* WebKit::core(WebKitDOMXPathResult*): assertion `coreObject' failed
> 
> ** (jumanji-debug:20791): CRITICAL **: WebKitDOMXPathResult* webkit_dom_document_evaluate(WebKitDOMDocument*, gchar*, WebKitDOMNode*, WebKitDOMXPathNSResolver*, gushort, WebKitDOMXPathResult*, GError**): assertion `converted_in_result' failed

The crash happens because you are just passing an empty GObject container without a core XPathResult inside. We need a way to construct proper empty XPathResults and/or fix the method to accept NULL objects.
------- Comment #3 From 2010-07-23 03:57:00 PST -------
Seems this is a little bigger, there are more functions which should be capable of parameters which accept NULL as values, e.g. like the pseudoElement of webkit_dom_dom_window_get_computed_style.

mrobinson pointed out that this is a problem in the perl script which generates the C code out of the idl files.
------- Comment #4 From 2010-08-28 06:53:41 PST -------
Created an attachment (id=65818) [details]
Patch that solves bug #42115

This patch (hack) solves this bug report by changing the CodeGeneratorGObject.pm script that it generates a proper implementation of the webkit_dom_document_evaluate function. 
It allows the in_result parameter to be NULL and checks further usage of it and the other converted values.
------- Comment #5 From 2010-08-28 06:59:12 PST -------
(From update of attachment 65818 [details])
Looks good, but needs a ChangeLog :)
------- Comment #6 From 2010-08-28 07:24:47 PST -------
Created an attachment (id=65822) [details]
Updated patch (Added Changelog entry)
------- Comment #7 From 2010-08-28 07:33:26 PST -------
Created an attachment (id=65823) [details]
Updated patch
------- Comment #8 From 2010-08-28 08:38:25 PST -------
Created an attachment (id=65825) [details]
Updated (Changed Changelog, ..)
------- Comment #9 From 2010-08-28 08:43:54 PST -------
(From update of attachment 65825 [details])
LGTM.
------- Comment #10 From 2010-08-28 08:59:29 PST -------
(From update of attachment 65825 [details])
Rejecting patch 65825 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Xan Lopez', u'--force']" exit_code: 2
Parsed 2 diffs from patch file(s).
patching file WebCore/ChangeLog
patch: **** malformed patch at line 24:  

patching file WebCore/bindings/scripts/CodeGeneratorGObject.pm

Full output: http://queues.webkit.org/results/3841070
------- Comment #11 From 2010-08-28 09:19:21 PST -------
Did someone hand-edit the patch?  That's the most common cause of this kind of failure.

webkit-patch apply-from-bug 42115

will repro the failure for you. :)
------- Comment #12 From 2010-08-28 09:37:14 PST -------
Created an attachment (id=65826) [details]
Updated patch (Genereated with git format-patch)
------- Comment #13 From 2010-08-28 09:39:24 PST -------
(From update of attachment 65826 [details])
Strike 2.
------- Comment #14 From 2010-08-28 09:57:04 PST -------
(From update of attachment 65826 [details])
Clearing flags on attachment: 65826

Committed r66308: <http://trac.webkit.org/changeset/66308>
------- Comment #15 From 2010-08-28 09:57:09 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #16 From 2010-08-28 10:23:33 PST -------
(In reply to comment #11)
> Did someone hand-edit the patch?  That's the most common cause of this kind of failure.
> 
> webkit-patch apply-from-bug 42115
> 
> will repro the failure for you. :)

The header of the WebCore/ChangeLog diff has an invalid range line "@@ -1,3 +1,21 @@". It should read "@@ -1,3 +1,19 @@". I'm not sure why someone would hand-edit the range.

Correcting the range line in the original patch I was able to apply the patch. I then ran svn-create-patch, and the output, including the range line in question, is correctly generated. I'm curious as to how attachment 65825 [details] <https://bugs.webkit.org/attachment.cgi?id=65825> was generated.
------- Comment #17 From 2010-08-28 10:27:22 PST -------
http://trac.webkit.org/changeset/66308 might have broken Qt Linux Release
------- Comment #18 From 2010-08-28 10:44:21 PST -------
(In reply to comment #16)
> (In reply to comment #11)
> > Did someone hand-edit the patch?  That's the most common cause of this kind of failure.
> > 
> > webkit-patch apply-from-bug 42115
> > 
> > will repro the failure for you. :)
> 
> The header of the WebCore/ChangeLog diff has an invalid range line "@@ -1,3 +1,21 @@". It should read "@@ -1,3 +1,19 @@". I'm not sure why someone would hand-edit the range.
>

err, I doubt that the range line was hand-edited. Most likely lines were removed by hand from the ChangeLog diff. That is, the ChangeLog diff was originally 21 lines long and was reduced to 19 lines by hand.
------- Comment #19 From 2010-08-28 13:56:36 PST -------
For clarification: I did not bear in mind that I changed a line in the Changelog right in the patch file, so in the end there is nothing wrong with the script. Sorry for the trouble