[Webkit-unassigned] [Bug 45898] editing/selection/context-menu-on-text.html fails on chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45898





--- Comment #7 from Kent Tamura <tkent at chromium.org>  2010-11-11 19:38:51 PST ---
(From update of attachment 73585)
View in context: https://bugs.webkit.org/attachment.cgi?id=73585&action=review

> WebKit/chromium/src/WebBindings.cpp:330
> +    for (size_t i = 0; i < data.size(); i++)

nit: we prefer ++i

> WebKitTools/DumpRenderTree/chromium/EventSender.cpp:741
> +    // These constant is based on Safari's context menu because tests are made for it.

"These constants are"?

> WebKitTools/DumpRenderTree/chromium/EventSender.cpp:770
> +    // Clears last context menu data because
> +    // we need to know if the context menu be requested after following mouse events.

Line breaking between "because" and "we" looks curious.  The first line is not so long and it can contain more words.

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:449
> +    OwnPtr<WebContextMenuData> given(new WebContextMenuData(contextMenuData));
> +    m_lastContextMenuData.swap(given);

Why not m_lastContextMenuData = adoptPtr(new WebContextmenuData(contextMenuData)) ?

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:455
> +    OwnPtr<WebContextMenuData> empty;
> +    m_lastContextMenuData.swap(empty);

Why not m_lastContextMenuData.clear()?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.