Bug 27763

Summary:

PopupMenuQt lacks proper separator

Product:

WebKit

Reporter:

Mike Fenton <mifenton>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

manyoso

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
Patch to replace separator and fix include order.	manyoso: review-
Improved Style patch	none
Remove extra brace from patch.	manyoso: review+

Mike Fenton

Reported 2009-07-28 06:37:42 PDT

As marked by a FIXME in PopupMenuQt.cpp, the separator item used is a hardcoded string of "---".

Attachments
Patch to replace separator and fix include order. (1.48 KB, patch) 2009-07-28 06:50 PDT, Mike Fenton	manyoso: review-	Details Formatted Diff Diff
Improved Style patch (1.95 KB, patch) 2009-07-28 07:22 PDT, Mike Fenton	no flags	Details Formatted Diff Diff
Remove extra brace from patch. (1.95 KB, patch) 2009-07-28 07:37 PDT, Mike Fenton	manyoso: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Mike Fenton

Comment 1 2009-07-28 06:50:35 PDT

Created attachment 33622 [details] Patch to replace separator and fix include order.

Adam Treat

Comment 2 2009-07-28 07:09:35 PDT

Comment on attachment 33622 [details] Patch to replace separator and fix include order. > if (client()->itemIsSeparator(i)) { > - //FIXME: better seperator item > - m_popup->insertItem(i, QString::fromLatin1("---")); > + m_popup->insertSeparator(i); > } else { Looks like you can remove the braces now from the if clause. cpplint didn't catch it? Also, you can go ahead and remove the #if 0 code. If it is important somehow, that's what we have revision control systems for :)

Mike Fenton

Comment 3 2009-07-28 07:22:11 PDT

Created attachment 33629 [details] Improved Style patch Thanks for the quick review. This patch removes the braces for both the if and the else as well as removes the unimplemented code block.

Mike Fenton

Comment 4 2009-07-28 07:37:13 PDT

Created attachment 33630 [details] Remove extra brace from patch.

Adam Treat

Comment 5 2009-07-28 07:41:50 PDT

Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/platform/qt/PopupMenuQt.cpp Committed r46483 M WebCore/ChangeLog M WebCore/platform/qt/PopupMenuQt.cpp r46483 = e84fb64b14709fba197a19c78e20c2076ee79ea2 (git-svn) No changes between current HEAD and refs/remotes/git-svn Resetting to the latest refs/remotes/git-svn http://trac.webkit.org/changeset/46483

Note You need to log in before you can comment on or make changes to this bug.