Bug 37320 - [Qt] tst_QWebFrame::popupFocus() fails
Summary: [Qt] tst_QWebFrame::popupFocus() fails
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P1 Blocker
Assignee: Yi Shen
URL:
Keywords: Qt
Depends on:
Blocks: 35784
  Show dependency treegraph
 
Reported: 2010-04-09 02:57 PDT by Simon Hausmann
Modified: 2010-04-12 07:12 PDT (History)
4 users (show)

See Also:


Attachments
proposal patch (1.30 KB, patch)
2010-04-09 11:58 PDT, Yi Shen
kenneth: review-
Details | Formatted Diff | Diff
patch (1.44 KB, patch)
2010-04-09 12:53 PDT, Yi Shen
kenneth: review-
Details | Formatted Diff | Diff
patch (1.45 KB, patch)
2010-04-09 13:28 PDT, Yi Shen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2010-04-09 02:57:08 PDT
PASS   : tst_QWebFrame::initTestCase()
FAIL!  : tst_QWebFrame::popupFocus() 'combo != 0' returned FALSE. ()
   Loc: [/home/shausman/src/webkit/trunk/WebKit/qt/tests/qwebframe/tst_qwebframe.cpp(2520)]
PASS   : tst_QWebFrame::cleanupTestCase()


Reproducible on Linux/X11. It does not appear to be timing related, it fails consistenly for me.
Comment 1 Yi Shen 2010-04-09 11:58:20 PDT
Created attachment 52972 [details]
proposal patch

The class WebCore::QWebPopup has been removed, so the test cases need to be updated also.
Comment 2 Luiz Agostini 2010-04-09 12:28:40 PDT
I can verify that this is the right fix. But you should elaborate more on what you are doing and how you fixed it in change log.
Comment 3 Kenneth Rohde Christiansen 2010-04-09 12:30:15 PDT
(In reply to comment #2)
> I can verify that this is the right fix. But you should elaborate more on what
> you are doing and how you fixed it in change log.

I agree, the ChangeLog entry doesn't fulfill the requirements of the projects, r- for that reason. Please provide a new patch with a better description and I will review it ASAP! Thanks :-)
Comment 4 Yi Shen 2010-04-09 12:53:45 PDT
Created attachment 52980 [details]
patch

updated the description in the changelog
Comment 5 Kenneth Rohde Christiansen 2010-04-09 13:19:29 PDT
Comment on attachment 52980 [details]
patch


> +
> +        The QWebPopup class has been moved & renamed, so tst_QWebFrame::popupFocus() should use the class name "QComboBox", rather than "WebCore::QWebPopup" to find the popup menu. 
> +

:-( Please keep the changelog at max 80-100 char per line as everyone else. sorry
Comment 6 Yi Shen 2010-04-09 13:28:39 PDT
Created attachment 52982 [details]
patch
Comment 7 Yi Shen 2010-04-09 13:29:22 PDT
It's my bad :) Thanks for the reviewing.
Comment 8 WebKit Commit Bot 2010-04-09 17:51:35 PDT
Comment on attachment 52982 [details]
patch

Clearing flags on attachment: 52982

Committed r57388: <http://trac.webkit.org/changeset/57388>
Comment 9 WebKit Commit Bot 2010-04-09 17:51:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Simon Hausmann 2010-04-12 07:11:53 PDT
Revision r57388 cherry-picked into qtwebkit-2.0 with commit 862268aeb7150d3795d05bfc05f661bb5c598a27