Bug 29682 – [Qt] QWebElement lacks hasFocus and SetFocus apis

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Joseph Ligman]

These APIs are needed for clients of QWebElement.

[Joseph Ligman]

Created attachment 40000 [details]
Proposed patch, adds setFocus, hasFocus to QWebElement

This patch adds setFocus and hasFocus Api to QWebElement. And a test case.

[Antonio Gomes]

good stuff

[Eric Seidel (no email)]

Are these standard Qt apis?  Please explain (ideally in the ChangeLog) why you're adding these, and why this particular API makes sense.

[Simon Hausmann]

Comment on attachment 40000 [details]
Proposed patch, adds setFocus, hasFocus to QWebElement

In principle this looks good. I think only a few small fixes are necessary.


>  /*!
> +    \since 4.6
> +    Returns true if this QWebElement has keyboard input focus; otherwise, returns false

In the documentation you can simply use "this element" instead of repeating the class name.
Just like in the other methods of this class :)

> +
> +    \sa setFocus()
> +*/
> +bool QWebElement::hasFocus() const
> +{
> +    if (m_element->document() && m_element->isFocusable())
> +        return m_element == m_element->document()->focusedNode();
> +    return false;
> +}

Please add a null pointer check for m_element.

> +/*!
> +    \since 4.6
> +    Gives keyboard input focus to this QWebElement
> +
> +    \sa hasFocus()
> +*/
> +void QWebElement::setFocus()
> +{
> +    if (m_element->document() && m_element->isFocusable())
> +        m_element->document()->setFocusedNode(m_element);
> +}

Same here :)

[Kenneth Rohde Christiansen]

> >  /*!
> > +    \since 4.6
> > +    Returns true if this QWebElement has keyboard input focus; otherwise, returns false
> 

The QWebElement class is new in 4.6 so I believe we should leave out the \since 4.6

[Joseph Ligman]

Created attachment 40127 [details]
Proposed patch, adds API setFocus, hasFocus to QWebElement

Fixed the comments and added the null pointer check for m_element

[Simon Hausmann]

Comment on attachment 40127 [details]
Proposed patch, adds API setFocus, hasFocus to QWebElement

r=me, thanks!

[WebKit Commit Bot]

Comment on attachment 40127 [details]
Proposed patch, adds API setFocus, hasFocus to QWebElement

Clearing flags on attachment: 40127

Committed r48805: <http://trac.webkit.org/changeset/48805>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.