the early return voids it
Created attachment 161471 [details] Patch
Comment on attachment 161471 [details] Patch Lets add this method to WebPagePrivate instead, so it does not became a public API.
Comment on attachment 161471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161471&action=review > Source/WebKit/blackberry/Api/WebPage.cpp:4100 > + // FIXME this checks if node search on inspector is enabled, though it might not be optimized nit1: FIXME: nit2: lacks a dot at the end.
Created attachment 161474 [details] Patch
Comment on attachment 161474 [details] Patch Clearing flags on attachment: 161474 Committed r127146: <http://trac.webkit.org/changeset/127146>
All reviewed patches have been landed. Closing bug.
Reopening to attach new patch.
Created attachment 162857 [details] Patch Re-opening since the old patch doesn't solve the issue
Comment on attachment 162857 [details] Patch I would feel more comfortable if we get some Inspector guys' review here (and also unprefix the bug title).
Remove BlackBerry from title since this fix requires cross-platform changes.
update component
Comment on attachment 162857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162857&action=review > Source/WebKit/blackberry/Api/WebPage.cpp:4111 > + if (InspectorInstrumentation::searchingForNode(d->m_mainFrame->page())) Could you provide more context on why it requires platform-specific handling for Blackberry? Also how is InspectorDOMAgent::handleMousePress called after this change?
Can this be implemented entirely on the WebCore level (as in the mouse case) so that all ports could use it?
Comment on attachment 162857 [details] Patch r- for the port-specific nature.
The event handling model of the BlackBerry is different than other ports and that is why there is port specific behaviour here. The issue in WebCore is that touch events fail to do 2 things when m_searchingForNode is true: 1) m_overlay->highlightedNode() is not set (in InspectorDOMAgent.cpp) 2) the touch event not prevented from firing There would need to be instrumentation added at the WebCore level to detect when touch events are fired and see if the InspectorDOMAgent is searching for a node.
Created attachment 166499 [details] Patch
Comment on attachment 166499 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166499&action=review The approach looks sane to me. Could you rebase the patch, since this one is not applicable to the tip of tree now? > Source/WebKit/blackberry/ChangeLog:3 > + node search does not work with elements on touch start listener I'm not quite familiar with the issue. Does this mean it is impossible to inspect elements that have the ontouchstart listener attached?
Created attachment 168063 [details] Patch
Comment on attachment 168063 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168063&action=review > Source/WebCore/inspector/InspectorInstrumentation.h:-111 > - static void mouseDidMoveOverElement(Page*, const HitTestResult&, unsigned modifierFlags); New names are confusing. I would leave them as is and add new signal "handleTouchEvent" instead. > Source/WebCore/page/EventHandler.cpp:3771 > + if (InspectorInstrumentation::platformEventHandled(m_frame->page())) There is no need to generate two signals, I'd rather call InspectorInstrumentation::handleTouchEvent and let inspector code handle it in case of inspect element mode.
As per the discussion with Pavel Feldman on IRC here's the plan: add InspectorInstrumentation::handleTouchEvent(Page*, Node*) and call this method only once in EventHandler::handleTouchEvent() avoiding 2 calls to InspectorInstruementation (which my current patch has)
Created attachment 168250 [details] Patch
Comment on attachment 168250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168250&action=review > Source/WebCore/ChangeLog:3 > + node search does not work with elements on touch start listener Web Inspector: prefix missing
Update bug title.
Created attachment 168252 [details] Patch
Comment on attachment 168252 [details] Patch Clearing flags on attachment: 168252 Committed r131083: <http://trac.webkit.org/changeset/131083>