Summary: | Change a logic for touching slider in "RangeInput::handleTouchEvent" | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Byung Jun Kim <bj1987.kim> | ||||||||||||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | NEW --- | ||||||||||||||||||
Severity: | Normal | CC: | benjamin, buildbot, commit-queue, joepeck, jonlee, lucas.de.marchi, rniwa, simon.fraser | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
Byung Jun Kim
2014-01-27 21:33:24 PST
Created attachment 222407 [details]
Patch
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. Created attachment 228143 [details]
Patch
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.
Created attachment 228144 [details]
Patch
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". 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.
Created attachment 228145 [details]
Patch
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. 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 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
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 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
Created attachment 228361 [details]
Patch
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". 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()* 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. |