Bug 127760 – Change a logic for touching slider in "RangeInput::handleTouchEvent"

• Status: NEW
• Component: WebKit EFL

[Byung Jun Kim]

If we touch slider and other side ,It weren't recognized as multi touch.
So change "TargetTouches" to "touches" in RangeInput::handleTouchEvent.

[Byung Jun Kim]

Created attachment 222407 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 222407 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222407&action=review

> Source/WebCore/html/RangeInputType.cpp:177
> -    TouchList* touches = event->targetTouches();
> +    TouchList* touches = event->touches();
>      if (touches->length() == 1) {
>          typedSliderThumbElement().setPositionFromPoint(touches->item(0)->absoluteLocation());
>          event->setDefaultHandled();

This is in non-iOS code, so I probably won't be the final reviewer, but why would you want to change from targetTouches to touches here?

Could you try to better describe the scenario you're fixing? I am having trouble understanding your comment in the ChangeLog.

[Byung Jun Kim]

Created attachment 228143 [details]
Patch

[WebKit Commit Bot]

Attachment 228143 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byung Jun Kim]

Created attachment 228144 [details]
Patch

[Byung Jun Kim]

In slider, nomal operation of Multi touch is changed from first touch.
So, thumb of slider should move only first touch.
But, If we touch thumb first and track second, slider moves from second touch.
Thumb and track is not same target. It isn't recognized multi touch because of "TargetTouch"
So we should change "touch" to "targetTouch".

[WebKit Commit Bot]

Attachment 228144 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/WebCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Byung Jun Kim]

Created attachment 228145 [details]
Patch

[Jinwoo Song]

Comment on attachment 228145 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228145&action=review

> Source/WebCore/ChangeLog:12
> +        So we should change "touch" to "targetTouch".

You'd better redescribe the change log as the sentences are not grammatical and it is hard to understand.

[Build Bot]

Comment on attachment 228145 [details]
Patch

Attachment 228145 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6225372660105216

New failing tests:
fast/forms/range/slider-multi-touch.html

[Build Bot]

Created attachment 228147 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 228145 [details]
Patch

Attachment 228145 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5021683916210176

New failing tests:
fast/forms/range/slider-multi-touch.html

[Build Bot]

Created attachment 228149 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Byung Jun Kim]

Created attachment 228361 [details]
Patch

[Byung Jun Kim]

If we touch thumb and track, slider should move from first touch. 
In current, slider is changed by second touch because of "targetTouches"
Thumb and track is not same target. So it isn't recognized multi touch.
So we should change "targetTouches" to "touches".

[Gyuyoung Kim]

Comment on attachment 228361 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228361&action=review

> Source/WebCore/ChangeLog:8
> +        If we touch thumb and track, slider should move from first touch. 

I don't still understand this description. So, I'm not 100% sure if my change is correct though, I modify this description as below,

When we touch thumb or track widget, *slider* on the thumb or track widgets is moved by first touch. However the slider under current implementation is worked by second touch. Because the slider is influenced by targetTouches() function as well. However, thumb and track widgets aren't same target widget, so those aren't able to be recognized by multiple touches. To fix this problem, we need to use *touches()* function instead of the *targetTouches()*

[Gyuyoung Kim]

Comment on attachment 228361 [details]
Patch

Cleared review? from attachment 228361 [details] so that this bug does not appear in http://webkit.org/pending-review.