Summary: | [Qt] - Context menu needs "Copy" as well when the selected text is a link. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Karsten Heimrich <karsten.heimrich> | ||||||||||
Component: | New Bugs | Assignee: | Manish Gupta <manish.5.gupta> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Enhancement | CC: | aparna.nand, benjamin, commit-queue, cshu, karsten.heimrich, mrobinson | ||||||||||
Priority: | P5 | Keywords: | EasyFix, Qt, QtTriaged | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | All | ||||||||||||
URL: | http://bugreports.qt.nokia.com/browse/QTBUG-11478 | ||||||||||||
Attachments: |
|
Description
Karsten Heimrich
2010-06-22 06:37:06 PDT
Please follow http://trac.webkit.org/wiki/QtWebKitBugs when reporing bugs (missing Qt keyword). Created attachment 81202 [details]
Patch for review
Modified to have Copy as a default option in context menu when the selection is a url.
Created attachment 81203 [details]
Patch for review with changelogs
Adding Copy to context menu
Comment on attachment 81203 [details] Patch for review with changelogs View in context: https://bugs.webkit.org/attachment.cgi?id=81203&action=review Shouldn't this code be part of ContextMenu.cpp in WebCore? > Source/WebKit/qt/ChangeLog:11 > + selection is a url. Adding this option to QWebPage Dot at the end of the sentence. Comment on attachment 81203 [details]
Patch for review with changelogs
r-
Unless there is a good reason, this kind of code should be in a place where other ports can use it. It can be #ifdef for Qt for now.
Created attachment 81251 [details]
Patch with corrected code
Added the fix in underlying layer.
Comment on attachment 81251 [details] Patch with corrected code View in context: https://bugs.webkit.org/attachment.cgi?id=81251&action=review Almost there I think. > Source/WebCore/ChangeLog:5 > + Copy menu option to be shown in context menu ? > Source/WebCore/ChangeLog:11 > + a link. Added a test in tst_qwebpage.cpp Dot at the end of the sentence or Kenneth will kill you. ;) > Source/WebCore/page/ContextMenuController.cpp:714 > +#if PLATFORM(QT) > + if (m_hitTestResult.isSelected()) > + appendItem(CopyItem, m_contextMenu.get()); > +#endif This seems reasonable. I did not remember ContextMenuController was so messy. :) > Source/WebKit/qt/ChangeLog:12 > + selects a link and opens context menu <Kenneth>Missing dot.</Kenneth> :) > Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2770 > + QContextMenuEvent event(QContextMenuEvent::Mouse, QPoint(71, 15)); > + view.page()->swallowContextMenuEvent(&event); > + view.page()->updatePositionDependentActions(QPoint(71, 15)); I don't like the hard coded position. That can make the test flaky depending on other conditions. Why not used QWebElement's rect to find the center of the link? > Source/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2777 > + foreach (QObject* child, view.children()) { > + contextMenu = qobject_cast<QMenu*>(child); > + if (contextMenu) > + break; > + } Can't you use view.findChildren<QMenu*>() here? Created attachment 81492 [details]
Patch for review
Added full stop in ChangeLog and edited test file.
Comment on attachment 81492 [details]
Patch for review
r=me :)
Comment on attachment 81492 [details] Patch for review Clearing flags on attachment: 81492 Committed r77906: <http://trac.webkit.org/changeset/77906> All reviewed patches have been landed. Closing bug. |