Bug 34573 – [Qt] layoutTestController.counterValueForElementById crashes for a nonexistent ID

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Shinichiro Hamaji]

This was added in Bug 33840.

[Shinichiro Hamaji]

Created attachment 48133 [details]
Patch v1

[Shinichiro Hamaji]

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.

[Simon Hausmann]

Comment on attachment 48133 [details]
Patch v1

Nice! Thanks :)

[Simon Hausmann]

(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 :)

[Kenneth Rohde Christiansen]

Didn't diego do the exact same patch today?

[Antonio Gomes]

(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 ***

[Shinichiro Hamaji]

Though the main change was done by Diego, I'll land the r+ed patch only with the crash fix.

[Shinichiro Hamaji]

Committed r58230: <http://trac.webkit.org/changeset/58230>