RESOLVED FIXED 82558
Toggling <input type="range"> readonly or disabled state while active breaks all click events
https://bugs.webkit.org/show_bug.cgi?id=82558
Summary Toggling <input type="range"> readonly or disabled state while active breaks ...
Joseph Pecoraro
Reported 2012-03-28 18:06:03 PDT
Created attachment 134458 [details] [REDUCTION] Test Case See attached reduction. If a user is actively dragging a slider and it toggles its readonly / disabled state buttons, other sliders, etc. stop working.
Attachments
[REDUCTION] Test Case (1.18 KB, text/html)
2012-03-28 18:06 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix 1: Stop Dragging when readonly / disabled state toggles (3.39 KB, patch)
2012-03-28 19:00 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix 2: Stop Dragging in the early bail (1.44 KB, patch)
2012-03-28 19:01 PDT, Joseph Pecoraro
no flags
[PATCH] Fix with test (6.30 KB, patch)
2012-03-28 20:11 PDT, Joseph Pecoraro
no flags
[TEXT] Expected Results Before Patch (930 bytes, text/plain)
2012-03-28 20:13 PDT, Joseph Pecoraro
no flags
Joseph Pecoraro
Comment 1 2012-03-28 18:07:25 PDT
My guess is that we miss a call to SliderThumbElement::stopDragging at some point to clear the global capturing mouse events node: frame->eventHandler()->setCapturingMouseEventsNode(0);
Joseph Pecoraro
Comment 2 2012-03-28 18:37:53 PDT
Yep, calling stopDragging() when fixes the issue. There are a couple different ways to fix this. I'm going to put up a few patches and let you know which one I recommend. I'm in the process of doing a build.
Joseph Pecoraro
Comment 3 2012-03-28 19:00:37 PDT
Created attachment 134472 [details] [PATCH] Proposed Fix 1: Stop Dragging when readonly / disabled state toggles
Joseph Pecoraro
Comment 4 2012-03-28 19:01:18 PDT
Created attachment 134474 [details] [PATCH] Proposed Fix 2: Stop Dragging in the early bail
WebKit Review Bot
Comment 5 2012-03-28 19:02:41 PDT
Attachment 134472 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:10: 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 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kent Tamura
Comment 6 2012-03-28 19:04:04 PDT
I like Proposed Fix 2 because of simplicity.
Joseph Pecoraro
Comment 7 2012-03-28 19:11:24 PDT
(In reply to comment #6) > I like Proposed Fix 2 because of simplicity. I wonder if we should take both. • Proposed Fix 1 - this would stop an active drag if the input is changed to disabled and back quickly. • Proposed Fix 2 - any early return should logically stop the drag. Should I spend time trying to make a test for this?
Joseph Pecoraro
Comment 8 2012-03-28 19:13:42 PDT
I see there is: LayoutTests/fast/forms/range/range-drag.html I'll see if I can make a test for this.
Joseph Pecoraro
Comment 9 2012-03-28 20:11:43 PDT
Created attachment 134483 [details] [PATCH] Fix with test Now with test! The question now is if we want the following scenario to work. 1. User starts dragging 2. <input> becomes disabled 3. <input> becomes enabled 4. User continues dragging. The test does not really test this scenario, it only really tests that mouse events don't stop working when a toggle during an active drag happens.
Joseph Pecoraro
Comment 10 2012-03-28 20:13:10 PDT
Created attachment 134484 [details] [TEXT] Expected Results Before Patch This is to show that the test does change from before and after. This is the before, note that the attempt to drag the second slider fails.
Kent Tamura
Comment 11 2012-03-29 00:29:12 PDT
Comment on attachment 134483 [details] [PATCH] Fix with test ok
WebKit Review Bot
Comment 12 2012-03-29 11:11:57 PDT
Comment on attachment 134483 [details] [PATCH] Fix with test Clearing flags on attachment: 134483 Committed r112547: <http://trac.webkit.org/changeset/112547>
WebKit Review Bot
Comment 13 2012-03-29 11:12:03 PDT
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.