Bug 29682

Summary: [Qt] QWebElement lacks hasFocus and SetFocus apis
Product: WebKit Reporter: Joseph Ligman <joseph.ligman>
Component: WebKit QtAssignee: Joseph Ligman <joseph.ligman>
Status: RESOLVED FIXED    
Severity: Enhancement CC: commit-queue, eric, kenneth, laszlo.gombos, tonikitoo, vestbo
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch, adds setFocus, hasFocus to QWebElement
hausmann: review-
Proposed patch, adds API setFocus, hasFocus to QWebElement none

Joseph Ligman
Reported 2009-09-23 07:00:35 PDT
These APIs are needed for clients of QWebElement.
Attachments
Proposed patch, adds setFocus, hasFocus to QWebElement (3.27 KB, patch)
2009-09-23 09:25 PDT, Joseph Ligman
hausmann: review-
Proposed patch, adds API setFocus, hasFocus to QWebElement (3.38 KB, patch)
2009-09-25 11:13 PDT, Joseph Ligman
no flags
Joseph Ligman
Comment 1 2009-09-23 09:25:40 PDT
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
Comment 2 2009-09-23 09:30:10 PDT
good stuff
Eric Seidel (no email)
Comment 3 2009-09-23 17:48:15 PDT
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 4 2009-09-25 04:15:12 PDT
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
Comment 5 2009-09-25 07:02:44 PDT
> > /*! > > + \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
Comment 6 2009-09-25 11:13:41 PDT
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 7 2009-09-27 14:23:50 PDT
Comment on attachment 40127 [details] Proposed patch, adds API setFocus, hasFocus to QWebElement r=me, thanks!
WebKit Commit Bot
Comment 8 2009-09-27 14:33:50 PDT
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
Comment 9 2009-09-27 14:33:54 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.