Bug 104369

Summary: [GTK] Expose HitTestResult::scrollbar() condition in API
Product: WebKit Reporter: Eduardo Lima Mitev <elima>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, gustavo, gyuyoung.kim, mrobinson, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 99591    
Bug Blocks:    
Attachments:
Description Flags
patch
cgarcia: review-, cgarcia: commit-queue-
previous patch with unit test
none
new patch correcting the ChangeLog
none
patch correcting the ChangeLog syntax none

Eduardo Lima Mitev
Reported 2012-12-07 07:32:33 PST
Specific UX behaviors require discriminating hits over elements' scrollbars, specially the WebView's main scrollbars. For example, any application behavior implemented on middle button, could break scrolling if the hit occurs over any scrollbars in the page.
Attachments
patch (9.69 KB, patch)
2012-12-07 08:14 PST, Eduardo Lima Mitev
cgarcia: review-
cgarcia: commit-queue-
previous patch with unit test (13.82 KB, patch)
2012-12-07 10:03 PST, Eduardo Lima Mitev
no flags
new patch correcting the ChangeLog (14.45 KB, patch)
2012-12-10 07:43 PST, Eduardo Lima Mitev
no flags
patch correcting the ChangeLog syntax (14.56 KB, patch)
2012-12-10 11:08 PST, Eduardo Lima Mitev
no flags
Eduardo Lima Mitev
Comment 1 2012-12-07 07:40:17 PST
Adding dependency on bug 99591.
Eduardo Lima Mitev
Comment 2 2012-12-07 08:14:44 PST
Carlos Garcia Campos
Comment 3 2012-12-07 08:29:47 PST
Comment on attachment 178221 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=178221&action=review Patch looks great, thanks! Setting r- just because it doesn't include unit tests. You can modify WebKitWebView/mouse-target test to check it. Use a form control containing scrollbars, because main scrollbars won't work due to bug #99591. > Source/WebKit2/Shared/WebHitTestResult.h:55 > + bool isOverScrollbar; I would use isScrollbar. > Source/WebKit2/Shared/WebHitTestResult.h:87 > + , isOverScrollbar(hitTestResult.scrollbar()) Ditto.
Eduardo Lima Mitev
Comment 4 2012-12-07 10:03:39 PST
Created attachment 178234 [details] previous patch with unit test This is the previous patch updated to include unit test, and to replace WebHitTestResult::isOverScrollbar by isScrollbar as suggested. Thanks for reviewing
WebKit Review Bot
Comment 5 2012-12-07 10:06:43 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Carlos Garcia Campos
Comment 6 2012-12-10 06:31:37 PST
Comment on attachment 178234 [details] previous patch with unit test View in context: https://bugs.webkit.org/attachment.cgi?id=178234&action=review > Source/WebKit2/ChangeLog:18 > + - Adds a new bool 'isScrollbar' to WebHitTestResult to > + carry the scrollbar() condition from Web process to UI process. > + - Adds a new flag WEBKIT_HIT_TEST_RESULT_CONTEXT_SCROLLBAR to > + WebKitHitTestResult context enum. > + - Adds the corresponding accessor context_is_scrollbar() to > + WebKitHitTestResult. > + - Adds the corresponding unit test cases to > + '/webkit2/WebKitWebView/mouse-target'. Please, move these below adding per function explanations instead. Sorry, that I didn't notice this in the previous patch.
Eduardo Lima Mitev
Comment 7 2012-12-10 07:43:10 PST
Created attachment 178550 [details] new patch correcting the ChangeLog >> Source/WebKit2/ChangeLog:18 >> + - Adds a new bool 'isScrollbar' to WebHitTestResult to >> + carry the scrollbar() condition from Web process to UI process. >> + - Adds a new flag WEBKIT_HIT_TEST_RESULT_CONTEXT_SCROLLBAR to >> + WebKitHitTestResult context enum. >> + - Adds the corresponding accessor context_is_scrollbar() to >> + WebKitHitTestResult. >> + - Adds the corresponding unit test cases to >> + '/webkit2/WebKitWebView/mouse-target'. > Please, move these below adding per function explanations instead. Sorry, > that I didn't notice this in the previous patch. New patch fixing this.
Eduardo Lima Mitev
Comment 8 2012-12-10 11:08:35 PST
Created attachment 178594 [details] patch correcting the ChangeLog syntax The ChangeLog syntax in previous patch was still wrong.
Carlos Garcia Campos
Comment 9 2012-12-10 11:22:54 PST
Comment on attachment 178594 [details] patch correcting the ChangeLog syntax Perfect, thanks!
WebKit Review Bot
Comment 10 2012-12-10 11:43:19 PST
Comment on attachment 178594 [details] patch correcting the ChangeLog syntax Clearing flags on attachment: 178594 Committed r137192: <http://trac.webkit.org/changeset/137192>
WebKit Review Bot
Comment 11 2012-12-10 11:43:24 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.