Bug 34573 - [Qt] layoutTestController.counterValueForElementById crashes for a nonexistent ID
Summary: [Qt] layoutTestController.counterValueForElementById crashes for a nonexisten...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-04 04:00 PST by Shinichiro Hamaji
Modified: 2010-04-25 09:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (5.63 KB, patch)
2010-02-04 04:42 PST, Shinichiro Hamaji
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 2010-02-04 04:00:19 PST
This was added in Bug 33840.
Comment 1 Shinichiro Hamaji 2010-02-04 04:42:19 PST
Created attachment 48133 [details]
Patch v1
Comment 2 Shinichiro Hamaji 2010-02-04 04:46:40 PST
Comment on attachment 48133 [details]
Patch v1

> diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp
> index aeb7a22..bc00f1b 100644
> --- a/WebKit/qt/Api/qwebframe.cpp
> +++ b/WebKit/qt/Api/qwebframe.cpp
> @@ -214,12 +214,23 @@ QString QWEBKIT_EXPORT qt_drt_counterValueForElementById(QWebFrame* qFrame, cons
>  {
>      Frame* frame = QWebFramePrivate::core(qFrame);
>      if (Document* document = frame->document()) {
> -        Element* element = document->getElementById(id);
> -        return WebCore::counterValueForElement(element);
> +        if (Element* element = document->getElementById(id))
> +            return WebCore::counterValueForElement(element);

This change is not related to this change, but I think it's small enough to fix with the main change. This change avoids crash when a nonexistent element ID is specified. If a reviewer considers this should be done in a separate change, I'm happy to split.
Comment 3 Simon Hausmann 2010-02-10 14:35:57 PST
Comment on attachment 48133 [details]
Patch v1

Nice! Thanks :)
Comment 4 Simon Hausmann 2010-02-10 14:36:15 PST
(In reply to comment #2)
> (From update of attachment 48133 [details])
> > diff --git a/WebKit/qt/Api/qwebframe.cpp b/WebKit/qt/Api/qwebframe.cpp
> > index aeb7a22..bc00f1b 100644
> > --- a/WebKit/qt/Api/qwebframe.cpp
> > +++ b/WebKit/qt/Api/qwebframe.cpp
> > @@ -214,12 +214,23 @@ QString QWEBKIT_EXPORT qt_drt_counterValueForElementById(QWebFrame* qFrame, cons
> >  {
> >      Frame* frame = QWebFramePrivate::core(qFrame);
> >      if (Document* document = frame->document()) {
> > -        Element* element = document->getElementById(id);
> > -        return WebCore::counterValueForElement(element);
> > +        if (Element* element = document->getElementById(id))
> > +            return WebCore::counterValueForElement(element);
> 
> This change is not related to this change, but I think it's small enough to fix
> with the main change. This change avoids crash when a nonexistent element ID is
> specified. If a reviewer considers this should be done in a separate change,
> I'm happy to split.

No worries and good catch :)
Comment 5 Kenneth Rohde Christiansen 2010-02-10 14:48:45 PST
Didn't diego do the exact same patch today?
Comment 6 Antonio Gomes 2010-02-10 19:01:50 PST
(In reply to comment #5)
> Didn't diego do the exact same patch today?

yes, see bug 34777, which has landed.

*** This bug has been marked as a duplicate of bug 34777 ***
Comment 7 Shinichiro Hamaji 2010-04-25 09:15:40 PDT
Though the main change was done by Diego, I'll land the r+ed patch only with the crash fix.
Comment 8 Shinichiro Hamaji 2010-04-25 09:17:14 PDT
Committed r58230: <http://trac.webkit.org/changeset/58230>