Set mouse pressed to false when the mouse is over the thumb of the scrollbar and change the variable name more clearly.
Created attachment 154912 [details] patch
Created attachment 155112 [details] patch2. Remove the unnecessary check statement.
Why do you only request cq ? Please request r? as well. And, did you check if this patch influences on layout test ? We need to verify it.
I'll request r+ for this patch. I think this does not need layout test because the scrollbar layout tests use the mockup scrollbar and this patch does not change the mockup scrollbar.
Created attachment 155113 [details] patch. request cq+ and r+.
(In reply to comment #4) > I'll request r+ for this patch. > I think this does not need layout test because the scrollbar layout tests use the mockup scrollbar and this patch does not change the mockup scrollbar. It seems to me this patch may influence on layout test result. If there are no differences in layout test result or no unskipped test cases, I can review this informally. See also, http://trac.webkit.org/wiki/WebKitEFLLayoutTest
okay, I'll check the if the regression is happened and let you know.
(In reply to comment #6) > (In reply to comment #4) > > I'll request r+ for this patch. > > I think this does not need layout test because the scrollbar layout tests use the mockup scrollbar and this patch does not change the mockup scrollbar. > > It seems to me this patch may influence on layout test result. If there are no differences in layout test result or no unskipped test cases, I can review this informally. > > See also, > http://trac.webkit.org/wiki/WebKitEFLLayoutTest I run the layout test and see no regression happened with my patch.
Comment on attachment 155113 [details] patch. I don't think this is the right place for a "fix". I might be remembering things wrongly, but I think the current behavior is by design. If you want to change it, you should write another EDC theme.
CC'ing a few people who might be more familiar with the history behind the current theme/behavior.
Bug 87950 is related to this one. Technically speaking, this one is even a duplicate of that.
(In reply to comment #9) > (From update of attachment 155113 [details]) > I don't think this is the right place for a "fix". > > I might be remembering things wrongly, but I think the current behavior is by design. If you want to change it, you should write another EDC theme. Actually, I'm curious about the current behavior is intended thing and by design. Because common users do not expect the text selection while scrolling the thumb.
(In reply to comment #11) > Bug 87950 is related to this one. Technically speaking, this one is even a duplicate of that. Yes, you are right and I could not find that in advance. But it seems that there is no progress in Bug 87950. If so, I'd like to follow that in this bug.
Created attachment 159011 [details] patch
I've changed the title of this patch according to the Kubo's comment. So I propose to change the scrollbar behavior not to select the text while moving the scrollbar thumb. As the event is handled in the ScrollbarEfl.cpp, the evas_object_propagate_events_set() is used not to propagate the event to webview.
Comment on attachment 159011 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=159011&action=review > Source/WebCore/ChangeLog:9 > + while moving the scrollbar thumb with the mouse. But It is not s/It/it/g > Source/WebCore/platform/efl/ScrollbarEfl.cpp:71 > + ScrollbarEfl* scrollbar = static_cast<ScrollbarEfl*>(data); This coding style change is not related to this bug. I think it would be good to change when this function is modified. > Source/WebCore/platform/efl/ScrollbarEfl.cpp:110 > + evas_object_propagate_events_set(object, EINA_FALSE); Please use standard false instead of EINA_FALSE.
Created attachment 159020 [details] patch
(In reply to comment #16) > (From update of attachment 159011 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=159011&action=review > > > Source/WebCore/ChangeLog:9 > > + while moving the scrollbar thumb with the mouse. But It is not > > s/It/it/g > Done. > > Source/WebCore/platform/efl/ScrollbarEfl.cpp:71 > > + ScrollbarEfl* scrollbar = static_cast<ScrollbarEfl*>(data); > > This coding style change is not related to this bug. I think it would be good to change when this function is modified. > Okay, I moved out the codes in this patch. > > Source/WebCore/platform/efl/ScrollbarEfl.cpp:110 > > + evas_object_propagate_events_set(object, EINA_FALSE); > > Please use standard false instead of EINA_FALSE. Done.
Comment on attachment 159020 [details] patch As kubo said, I also think we can decide if this patch is landed after getting original authors's opinions. Rafael, Demarchi and Leandro, could you guys take a look this patch ?
(In reply to comment #19) > (From update of attachment 159020 [details]) > As kubo said, I also think we can decide if this patch is landed after getting original authors's opinions. > > Rafael, Demarchi and Leandro, could you guys take a look this patch ? Rafael, Demarchi and Leandro, don't you have any feedback?
Comment on attachment 159020 [details] patch I wonder if this patch is still valid.
I'll make this bug invalid as there is no agreement and discussions any more. Instead, I'll start to look into the WebKit2/EFL scrollbar implementation and this issue may be discussed in that thread.