[webkit-reviews] review denied: [Bug 47203] [Qt] No way to programatically specify element to focus the inspector : [Attachment 70202] Updated patch according to suggestions in comment #2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 26 15:30:37 PDT 2011


Andreas Kling <kling at webkit.org> has denied Eugene Ostroukhov
<eostroukhov at gmail.com>'s request for review:
Bug 47203: [Qt] No way to programatically specify element to focus the
inspector
https://bugs.webkit.org/show_bug.cgi?id=47203

Attachment 70202: Updated patch according to suggestions in comment #2
https://bugs.webkit.org/attachment.cgi?id=70202&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70202&action=review

> WebKit/qt/Api/qwebpage.cpp:2136
> -		   d->getOrCreateInspector(); // Make sure the inspector is
created
> -		   d->inspector->show(); // The inspector is expected to be
shown on inspection
> -		  
d->page->inspectorController()->inspect(d->hitTestResult.d->innerNonSharedNode.
get());
> -	       }
> +	       if (!d->hitTestResult.isNull())
> +		   inspect(d->hitTestResult.element());

This breaks inspecting a text node from the right-click menu, since text nodes
cannot be represented by QWebElement.

> WebKit/qt/Api/qwebpage.cpp:2162
> +/*!
> +    This function focuses the inspector on the specified \a element.
> +    The inspector becomes visible if it wasn't already.
> + */

Focuses is not exactly the right word. It should also mention that it may open
the inspector if it's not already open.
Also needs a \since 4.8 doc tag.

> WebKit/qt/Api/qwebpage.cpp:2166
> +    d->getOrCreateInspector(); // Make sure the inspector is created

No need for the comment.

> WebKit/qt/Api/qwebpage.h:351
> +    void inspect(const QWebElement& element);

This API is not good enough. We also want to be able to inspect() text nodes.
There is currently no QtWebKit class that wraps a WebCore::Node, so we'll need
that first.
There was a discussion on webkit-qt a while ago about this.


More information about the webkit-reviews mailing list