Last Comment Bug 719320 - Implement DOM3 wheel event
: Implement DOM3 wheel event
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal with 4 votes (vote)
: mozilla17
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
http://dev.w3.org/2006/webapi/DOM-Lev...
Depends on: 1121480 672175 674477 731878 739760 782175 782181 782190 782552 782583 782739 782853 786120 788401 801101 814303 829952 848628
Blocks: 725851 310003 422132 659071 741968 782143 1191293
  Show dependency treegraph
 
Reported: 2012-01-18 20:03 PST by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2015-08-24 13:36 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (2.47 KB, text/html)
2012-01-18 20:03 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details
part.1 Add DOM3 WheelEvent (59.65 KB, patch)
2012-03-16 04:37 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
Use float for deltas (4.36 KB, patch)
2012-03-18 01:59 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Review
Use float for deltas (6.51 KB, patch)
2012-03-18 02:05 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Review
Use float for deltas (7.58 KB, patch)
2012-03-18 05:51 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (71.91 KB, patch)
2012-04-27 02:49 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (15.04 KB, patch)
2012-04-27 02:50 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (18.46 KB, patch)
2012-04-27 02:50 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs (30.32 KB, patch)
2012-04-27 02:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.5 Don't separate wheel action prefs by direction (17.16 KB, patch)
2012-04-27 02:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.6 Separate pixel delta value accumulation code (9.26 KB, patch)
2012-04-27 02:52 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (9.94 KB, patch)
2012-04-27 02:52 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler (82.75 KB, patch)
2012-04-27 02:53 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (14.82 KB, patch)
2012-04-27 02:53 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (273.88 KB, patch)
2012-04-27 02:54 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.11 Fix new test failures (31.57 KB, patch)
2012-04-27 02:54 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (69.94 KB, patch)
2012-04-27 02:55 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (8.16 KB, patch)
2012-04-27 02:55 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (18.55 KB, patch)
2012-04-27 02:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt (3.12 KB, patch)
2012-04-27 02:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2 (3.30 KB, patch)
2012-04-27 02:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils (3.88 KB, patch)
2012-04-27 02:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.18 Clean up legacy mouse scroll event (16.94 KB, patch)
2012-04-27 02:58 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (71.50 KB, patch)
2012-04-27 04:45 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (71.63 KB, patch)
2012-05-07 20:38 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.2 -w (9.65 KB, patch)
2012-05-07 20:44 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (61.88 KB, patch)
2012-05-17 01:52 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (14.78 KB, patch)
2012-05-17 01:53 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (273.82 KB, patch)
2012-05-17 01:54 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (61.87 KB, patch)
2012-05-23 18:19 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
jst: superreview+
Details | Diff | Review
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (15.05 KB, patch)
2012-05-23 18:41 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (18.33 KB, patch)
2012-05-23 19:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler (82.69 KB, patch)
2012-05-24 01:41 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (273.82 KB, patch)
2012-05-24 01:41 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (18.56 KB, patch)
2012-05-24 01:42 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.18 Clean up legacy mouse scroll event (16.88 KB, patch)
2012-05-24 01:42 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.6 Separate pixel delta value accumulation code (9.87 KB, patch)
2012-05-31 16:17 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-1 Drop legacy mouse scroll events from IPC (4.29 KB, patch)
2012-05-31 16:19 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (8.86 KB, patch)
2012-05-31 16:20 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (17.49 KB, patch)
2012-05-31 16:24 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (33.71 KB, patch)
2012-05-31 16:27 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed (20.78 KB, patch)
2012-05-31 16:30 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.8-6 Init intDeltaX and intDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device (10.80 KB, patch)
2012-05-31 16:33 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (13.83 KB, patch)
2012-05-31 16:36 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.8-8 widget should be able to speicify the scroll type (6.63 KB, patch)
2012-05-31 16:38 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-9 Handle WheelEvent.deltaX in ESM (5.34 KB, patch)
2012-05-31 16:39 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-10 Remove the code handling legacy mouse events in layout (2.05 KB, patch)
2012-05-31 16:39 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (61.79 KB, patch)
2012-06-20 00:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (r=smaug) (15.05 KB, patch)
2012-06-20 00:01 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (r=smaug) (18.33 KB, patch)
2012-06-20 00:02 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs (30.32 KB, patch)
2012-06-20 00:07 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.5 Don't separate wheel action prefs by direction (r=smaug) (17.13 KB, patch)
2012-06-20 00:08 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
masayuki: review-
Details | Diff | Review
part.6 Separate pixel delta value accumulation code (r=smaug) (10.20 KB, patch)
2012-06-20 00:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug) (9.95 KB, patch)
2012-06-20 00:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-1 Drop legacy mouse scroll events from IPC (r=smaug) (4.30 KB, patch)
2012-06-20 00:10 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug) (8.87 KB, patch)
2012-06-20 00:10 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug) (17.50 KB, patch)
2012-06-20 00:11 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug) (33.71 KB, patch)
2012-06-20 00:12 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed (20.92 KB, patch)
2012-06-20 00:13 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device (r=smaug) (11.23 KB, patch)
2012-06-20 00:14 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (14.46 KB, patch)
2012-06-20 00:18 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-8 widget should be able to speicify the scroll type (r=smaug) (6.66 KB, patch)
2012-06-20 00:19 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (5.35 KB, patch)
2012-06-20 00:19 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-10 Remove the code handling legacy mouse events in layout (r=smaug) (2.06 KB, patch)
2012-06-20 00:20 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (14.81 KB, patch)
2012-06-20 00:21 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (274.55 KB, patch)
2012-06-20 00:23 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.11 Fix new test failures (31.60 KB, patch)
2012-06-20 00:23 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (69.90 KB, patch)
2012-06-20 00:24 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (8.16 KB, patch)
2012-06-20 00:24 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (18.56 KB, patch)
2012-06-20 00:25 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt (3.12 KB, patch)
2012-06-20 00:26 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2 (3.30 KB, patch)
2012-06-20 00:26 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils (3.88 KB, patch)
2012-06-20 00:27 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.18 Clean up legacy mouse scroll event (16.88 KB, patch)
2012-06-20 00:29 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug) (17.56 KB, patch)
2012-06-20 06:26 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed (21.15 KB, patch)
2012-06-20 06:27 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.8-6 Init lineOrPageDeltaX and lineOrPageDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device (r=smaug) (11.39 KB, patch)
2012-06-20 06:28 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
masayuki: review-
Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (14.66 KB, patch)
2012-06-20 06:29 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review-
Details | Diff | Review
part.8-8 widget should be able to speicify the scroll type (r=smaug) (6.80 KB, patch)
2012-06-20 06:29 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (5.36 KB, patch)
2012-06-20 06:30 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (14.89 KB, patch)
2012-06-20 06:31 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (278.81 KB, patch)
2012-06-20 06:31 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.11 Fix new test failures (31.75 KB, patch)
2012-06-20 06:32 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (70.08 KB, patch)
2012-06-20 06:33 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (8.20 KB, patch)
2012-06-20 06:33 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (18.57 KB, patch)
2012-06-20 06:34 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt (3.13 KB, patch)
2012-06-20 06:34 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
karlt: review+
Details | Diff | Review
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2 (3.31 KB, patch)
2012-06-20 06:35 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils (3.89 KB, patch)
2012-06-20 06:36 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (61.88 KB, patch)
2012-07-05 00:16 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (r=smaug) (14.90 KB, patch)
2012-07-05 00:19 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
jst: superreview+
Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug) (19.14 KB, patch)
2012-07-05 00:20 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
smichaud: review+
Details | Diff | Review
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2 (3.36 KB, patch)
2012-07-05 00:21 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
daveryeo: review+
Details | Diff | Review
part.18 Clean up legacy mouse scroll event (16.81 KB, patch)
2012-07-05 00:22 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (61.88 KB, patch)
2012-07-05 23:50 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (33.30 KB, patch)
2012-07-05 23:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.5 Redesign mouse wheel action prefs (23.85 KB, patch)
2012-07-06 00:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.6 Separate pixel delta value accumulation code (r=smaug) (10.75 KB, patch)
2012-07-06 00:12 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug) (9.97 KB, patch)
2012-07-06 00:23 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug) (34.03 KB, patch)
2012-07-06 01:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed (24.20 KB, patch)
2012-07-06 01:28 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs (18.04 KB, patch)
2012-07-06 03:17 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (16.21 KB, patch)
2012-07-06 04:10 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (4.02 KB, patch)
2012-07-06 06:08 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-11 Cancel applying delta multipliers from overflowDelta (3.57 KB, patch)
2012-07-06 06:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (61.89 KB, patch)
2012-07-10 23:27 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug) (34.12 KB, patch)
2012-07-10 23:33 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed (23.79 KB, patch)
2012-07-10 23:34 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs (18.12 KB, patch)
2012-07-10 23:40 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (16.23 KB, patch)
2012-07-10 23:43 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (5.39 KB, patch)
2012-07-10 23:45 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-11 Cancel applying delta multipliers from overflowDelta (3.55 KB, patch)
2012-07-10 23:47 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (320.93 KB, patch)
2012-07-10 23:53 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.11 Fix new test failures (32.44 KB, patch)
2012-07-10 23:55 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (r=smaug) (69.51 KB, patch)
2012-07-10 23:56 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
jmathies: review+
Details | Diff | Review
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (5.16 KB, patch)
2012-07-10 23:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
karlt: review+
Details | Diff | Review
part.18 Clean up legacy mouse scroll event (10.28 KB, patch)
2012-07-10 23:59 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (33.41 KB, patch)
2012-07-18 02:14 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.5 Redesign mouse wheel action prefs (24.02 KB, patch)
2012-07-18 02:15 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (5.43 KB, patch)
2012-07-18 02:16 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (61.87 KB, patch)
2012-07-24 15:36 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (r=smaug) (33.42 KB, patch)
2012-07-24 15:37 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.5 Redesign mouse wheel action prefs (r=smaug) (24.09 KB, patch)
2012-07-24 15:38 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-1 Drop legacy mouse scroll events from IPC (r=smaug) (5.88 KB, patch)
2012-07-24 15:39 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug) (7.55 KB, patch)
2012-07-24 15:40 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug) (34.11 KB, patch)
2012-07-24 15:41 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed (23.79 KB, patch)
2012-07-24 15:45 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs (18.13 KB, patch)
2012-07-24 15:46 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (16.20 KB, patch)
2012-07-24 15:48 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug) (5.43 KB, patch)
2012-07-24 15:50 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (320.93 KB, patch)
2012-07-24 15:52 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
Details | Diff | Review
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (r=karlt) (4.69 KB, patch)
2012-07-24 15:53 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst) (60.66 KB, patch)
2012-08-08 05:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (r=smaug) (15.06 KB, patch)
2012-08-08 05:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (r=smaug) (18.34 KB, patch)
2012-08-08 05:58 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (r=smaug) (33.43 KB, patch)
2012-08-08 05:59 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.5 Redesign mouse wheel action prefs (r=smaug) (24.09 KB, patch)
2012-08-08 05:59 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.6 Separate pixel delta value accumulation code (r=smaug) (10.76 KB, patch)
2012-08-08 06:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug) (9.98 KB, patch)
2012-08-08 06:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug) (7.55 KB, patch)
2012-08-08 06:01 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug) (17.57 KB, patch)
2012-08-08 06:01 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug) (34.12 KB, patch)
2012-08-08 06:02 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed (r=smaug) (23.81 KB, patch)
2012-08-08 06:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs (r=smaug) (18.21 KB, patch)
2012-08-08 06:03 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-7 Implement the other default actions of WheelEvent (r=smaug) (16.21 KB, patch)
2012-08-08 06:04 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-8 widget should be able to speicify the scroll type (r=smaug) (6.80 KB, patch)
2012-08-08 06:04 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.8-10 Remove the code handling legacy mouse events in layout (r=smaug) (2.06 KB, patch)
2012-08-08 06:05 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (r=smaug) (14.89 KB, patch)
2012-08-08 06:06 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (r=smaug) (320.94 KB, patch)
2012-08-08 06:07 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (r=smaug+jimm) (69.67 KB, patch)
2012-08-08 06:07 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug+smichaud) (19.09 KB, patch)
2012-08-08 06:08 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils (r=smaug) (3.88 KB, patch)
2012-08-08 06:09 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.18 Clean up legacy mouse scroll event (r=smaug) (10.29 KB, patch)
2012-08-08 06:10 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
testcase #2 (2.16 KB, text/html)
2012-08-08 23:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-01-18 20:03:28 PST
Created attachment 589763 [details]
testcase

I think that we can relatively easily implement D3E's WheelEvent.

All widgets should dispatch a new internal event for each native event. And ESM should generate old mouse scroll event and pixel scroll event for compatibility.

Now, I'm working on refactoring the complex implementation on Windows (bug 672175). After that, maybe, I can fix this bug.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-28 18:05:39 PST
Smaug:

There are only the DOM events class whose names are nsDOM*Event. Which is better for the new D3E wheel event, nsDOMWheelEvent or mozilla::events::DOMWheelEvent (or mozilla::DOMWheelEvent)?
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-02-28 21:49:31 PST
do we have mozilla::events for anything else? If not, lets not add new namespaces.
mozilla::dom should be fine.
So, perhaps mozilla::dom::DOMWheelEvent
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-28 22:14:35 PST
OK, sounds reasonable. Thanks.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-16 04:36:49 PDT
Hmm, I have a trouble.

DOM 3 Events defines the type of delta values as float. I don't find the definition of float in DOM spec.

Our Javascript engine always uses double for float. Therefore, if we used float for the delta attributes, it would cause error of casting float to double. E.g., following function returned false.

function () {
  var event = new WheelEvent("wheel", { deltaX: 1.0 });
  return (event.deltaX == 1.0);
}

the actual deltaX may be 1.000000xxxxxxxxx (x means random number).

When I defined them as double, the problem has gone. So, I think that we need to use double for them.
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-16 04:37:40 PDT
Created attachment 606531 [details] [diff] [review]
part.1 Add DOM3 WheelEvent
Comment 6 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-16 05:11:51 PDT
Yeah, double sounds good.
Comment 7 Masatoshi Kimura [:emk] 2012-03-17 20:24:16 PDT
Comment on attachment 606531 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

> -    elif realtype.count("PRUint16"):
> +    elif realtype.count("PRUint32") or realtype.count("PRUint16"):
>          fd.write("    uint32_t u;\n")
>          fd.write("    NS_ENSURE_STATE(JS_ValueToECMAUint32(aCx, v, &u));\n")
>          fd.write("    aDict.%s = u;\n" % a.name)
Extra assignment is not required for PRUint32.

> +    elif realtype.count("double"):
> +        fd.write("    MOZ_ALWAYS_TRUE(JS_ValueToNumber(aCx, v, &aDict.%s));\n" % a.name)
Use NS_ENSURE_STATE. JS_ValueToNumber can fail.

It should be:
+    elif realtype.count("PRUint32"):
+        fd.write("    NS_ENSURE_STATE(JS_ValueToECMAUint32(aCx, v, &aDict.%s));\n" % a.name)
+    elif realtype.count("double"):
+        fd.write("    NS_ENSURE_STATE(JS_ValueToNumber(aCx, v, &aDict.%s));\n" % a.name)

See codegen.py, qsgen.py, and JSData2Native to verify how to convert various types.
https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/codegen.py#98
https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/qsgen.py#428
https://mxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCConvert.cpp#395
Comment 8 Masatoshi Kimura [:emk] 2012-03-18 01:59:04 PDT
Created attachment 606950 [details] [diff] [review]
Use float for deltas

> E.g., following function returned false.
> function () {
>   var event = new WheelEvent("wheel", { deltaX: 1.0 });
>   return (event.deltaX == 1.0);
> }
I couldn't reproduce it with the attached patch.

> var event = document.createEvent("WheelEvent");
> event.initWheelEvent("wheel", false, false, null, null, 0, 0, 0, 0, 0, null, null, 0.1, 0, 0, 0);
> event.deltaX == 0.1
Indeed returns false. However, I think we should follow the spec here because:
- It's consistent with IE9/10.
- Sooner or later, JS programmers will have to know about floating-point numbers precision. Single-precision floating-point numbers will be used more and more in specs. (WebGL, Typed Array, etc.)
Comment 9 Masatoshi Kimura [:emk] 2012-03-18 02:05:36 PDT
Created attachment 606951 [details] [diff] [review]
Use float for deltas

Forgot qrefresh
Comment 10 Masatoshi Kimura [:emk] 2012-03-18 02:17:36 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #4)
> DOM 3 Events defines the type of delta values as float. I don't find the
> definition of float in DOM spec.
> 
> Our Javascript engine always uses double for float.
This is not implementation defined but well defined in ECMA-262 and Web IDL. It is an expected behavior for the following function to return false:
function () {
  var event = new WheelEvent("wheel", { deltaX: 0.1 });
  return (event.deltaX == 0.1);
}
Comment 11 Masatoshi Kimura [:emk] 2012-03-18 05:51:36 PDT
Created attachment 606965 [details] [diff] [review]
Use float for deltas

Test fix added
Comment 12 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-18 07:36:57 PDT
Kimura-san, thank you for your work.

I think IE 9 doesn't use float for the delta values. Perhaps, I suggested the type of delta values after IE 9 was released (at implementing high resolution mouse wheel event handling on Windows). I've not tested on IE 10.

And I don't think that using float is better than using double because looks like using Float32Array is pretty redundant... Hmm...
Comment 13 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-18 07:48:36 PDT
So, there are two options:

1. Use float because comparing delta value is not real situation.

2. Use double for avoiding confusion of web developers.
Comment 14 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-18 08:00:57 PDT
float vs. double issue was discussed long ago in a DOM Events conf call, but I don't recall now
what was decided and why.
http://www.w3.org/2008/webapps/track/issues/177

If no one else implement it as a float, we could still change the spec to use double.
Comment 15 Masatoshi Kimura [:emk] 2012-03-18 08:13:01 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #12)
> I think IE 9 doesn't use float for the delta values.
Ah, you are right. IE9 uses integer for delta values.
> I've not tested on IE 10.
In my testing, IE10 uses single-precision floating-point number for delta.

> And I don't think that using float is better than using double because looks
> like using Float32Array is pretty redundant... Hmm...
In general, floating-point values should not be compared with expressions such as "f1 == f2". It should be "Math.abs(f1 - f2) < 0.000001" if required. Also I think it's unlikely to compare delta values in real-world.

(In reply to Olli Pettay [:smaug] from comment #14)
> If no one else implement it as a float, we could still change the spec to
> use double.
IE10 already implements it as a float.
Comment 16 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-18 08:37:27 PDT
Well, IE10 is not released yet ;)
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-18 09:21:05 PDT
(In reply to Masatoshi Kimura [:emk] from comment #10)
> > Our Javascript engine always uses double for float.
> This is not implementation defined but well defined in ECMA-262 and Web IDL.

If JS's fractional number and Web IDL's double are always same, I think that it doesn't make sense to use "float" for delta values. Using double makes simpler Web application's code. Additionally, the size difference isn't problem because typically allocated event is deleted after it's dispatched. So, it seems that using double in D3E spec is better. I have no idea about its demerit.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-23 01:20:07 PDT
I'm rewriting ESM. I think that we should discard *all* mousewheel settings which are handled by ESM.

Nowadays, replacing scrolling delta values with pref values doesn't make sense. Coming delta values are not same always. On the other hand, I understand some platform's users want to customize it like Linux.

I think that each widget should implement the prefs for overriding their system settings, like Windows.
http://mxr.mozilla.org/mozilla-central/source/widget/windows/WinMouseScrollHandler.cpp#1175

So, I'll remove all mousewheel.*.sysnumlines, mousewheel.*.numlines.

And I need to change the .action prefs. They should only specify "scroll", "history", "zoom" or "none". I.e., scrolling by lines, pixels and pages shouldn't be specified by this pref.

And the .action shouldn't be separated by the difference between vertical or horizontal because new wheel event can handle both events at once.
Comment 19 Daniel Friesen 2012-04-03 19:21:46 PDT
Is there enough information from the system to differentiate devices?

Looking at WheelEvent it looks like a trackpad or smooth scroll wheel may want to use DOM_DELTA_PIXEL while a mouse with a non-smooth scroll wheel may want to use DOM_DELTA_LINE or DOM_DELTA_PAGE.
Comment 20 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-03 20:39:22 PDT
(In reply to Daniel Friesen from comment #19)
> Is there enough information from the system to differentiate devices?

No. It's difficult for native applications too on most platforms.

> Looking at WheelEvent it looks like a trackpad or smooth scroll wheel may
> want to use DOM_DELTA_PIXEL while a mouse with a non-smooth scroll wheel may
> want to use DOM_DELTA_LINE or DOM_DELTA_PAGE.

It depends on the platform's native event. On Mac, some pointing devices provide delta values in pixels, the others provide it in lines. On Windows, the delta value means lines or pages (but I'm not sure for tablet PC). On Linux, it always means lines. Even if it means lines, it may cause smooth scrolling, e.g., on Windows. Please be aware that the delta values are not integer.
Comment 21 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:49:40 PDT
Created attachment 618959 [details] [diff] [review]
part.1 Add DOM3 WheelEvent
Comment 22 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:50:10 PDT
Created attachment 618960 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText()
Comment 23 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:50:40 PDT
Created attachment 618961 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events
Comment 24 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:51:06 PDT
Created attachment 618962 [details] [diff] [review]
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs
Comment 25 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:51:37 PDT
Created attachment 618963 [details] [diff] [review]
part.5 Don't separate wheel action prefs by direction
Comment 26 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:52:16 PDT
Created attachment 618964 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code
Comment 27 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:52:48 PDT
Created attachment 618966 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:53:22 PDT
Created attachment 618967 [details] [diff] [review]
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler
Comment 29 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:53:49 PDT
Created attachment 618968 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests
Comment 30 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:54:24 PDT
Created attachment 618969 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 31 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:54:48 PDT
Created attachment 618971 [details] [diff] [review]
part.11 Fix new test failures
Comment 32 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:55:10 PDT
Created attachment 618972 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows
Comment 33 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:55:38 PDT
Created attachment 618973 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK
Comment 34 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:56:06 PDT
Created attachment 618974 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa
Comment 35 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:56:31 PDT
Created attachment 618975 [details] [diff] [review]
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt
Comment 36 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:56:58 PDT
Created attachment 618976 [details] [diff] [review]
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2
Comment 37 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:57:27 PDT
Created attachment 618977 [details] [diff] [review]
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils
Comment 38 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:58:07 PDT
Created attachment 618978 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event
Comment 39 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 02:59:26 PDT
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=7df866b71e58

I'll request reviews after I'll be back.
Comment 40 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-04-27 04:45:33 PDT
Created attachment 618990 [details] [diff] [review]
part.1 Add DOM3 WheelEvent
Comment 41 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 20:38:45 PDT
Created attachment 621863 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

Updated for latest trunk.

This patch adds mozilla::dom::DOMWheelEvent in content/events/src and mozilla::widget::WheelEvent in widget.

And also, it adds "case NS_WHEEL_EVENT" next to "case NS_MOUSE_SCROLL_EVENT".

The "onwheel" attribute is for XUL for now.
Comment 42 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 20:44:03 PDT
Comment on attachment 618960 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText()

This patch separates nsEventStateManager::DoScrollText() to 2 methods. One is just computing the scroll target frame. The other modifiers the mouse wheel transaction and scroll the given frame actually. I'll attach -w diff.
Comment 43 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 20:44:32 PDT
Created attachment 621864 [details] [diff] [review]
part.2 -w
Comment 44 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 20:52:49 PDT
Comment on attachment 618961 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events

The scroll target for default action is computed with some additional checks such as whether it's scrollable for the direction, whether it's not overflow:hidden and honoring mouse wheel transaction. On the other hand, legacy event's detail (delta value) is computed from nearest scrollable element.

This patch makes nsEventStateManager::ComputeScrollTarget() usable for computing the nearest scrollable element for computing the detail value of legacy mouse scroll events.

The actual scroll amount is computed by nsEventStateManager::GetScrollAmount().
Comment 45 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 21:11:40 PDT
Comment on attachment 618962 [details] [diff] [review]
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs

This patch removes the prefs for customizing legacy mouse scroll event's detail value.

D3E WheelEvent has 3 delta modes such as PIXEL, LINE and PAGE. And the delta values are double. So, we will fully support high-resolution scrolling.

So, specifying delta values don't make sense for now. But user may want to customize the scroll amount actually. I think that it should be possible on each widget. Therefore, I think these prefs are not necessary for ESM at least.

On the other hand, ESM should be possible to change the delta value direction ( *= -1) because it's impossible on every platform. If somebody customized the prefs as negative values, it would be a big regression for them if we don't provide these prefs.

And also, I'd like to change the rules for naming mousewheel prefs. Currently, we're deciding a used pref with following code.

> 362   if (aEvent->IsShift()) {
> 363     aPref.Append(withshift);
> 364   } else if (aEvent->IsControl()) {
> 365     aPref.Append(withcontrol);
> 366   } else if (aEvent->IsAlt()) {
> 367     aPref.Append(withalt);
> 368   } else if (aEvent->IsMeta()) {
> 369     aPref.Append(withmetakey);
> 370   } else {
> 371     aPref.Append(withno);
> 372   }

This code doesn't make sense if two or more modifiers are pressed.

I think that only when a modifier key is pressed, it takes a special action. Otherwise, the "default" action should be performed.

And changing the naming rules allow to sync profiles of both ESR10 and newer Fx.
Comment 46 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 21:23:57 PDT
Comment on attachment 618963 [details] [diff] [review]
part.5 Don't separate wheel action prefs by direction

D3E WheelEvent can indicate oblique scrolling. On Mac, the native event can indicate oblique scrolling too. So, it doesn't make sense we keep two separated action prefs for vertical and horizontal. We should provide only one pref for actions.

And also, it shouldn't be able to specify the deltaMode for scrolling action. It doesn't make sense.
Comment 47 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 21:27:34 PDT
Comment on attachment 618964 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code

This patch separates the delta accumulation code from PreHandleEvent(). It will be useful for D3E wheel event's pixel scroll events.
Comment 48 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 21:29:11 PDT
Comment on attachment 618966 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM

NS_QUERY_SCROLL_TARGET_INFO event isn't necessary after we implement D3E WheelEvent. This patch drops the handler from ESM (from widget is preformed later).
Comment 49 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-07 21:44:09 PDT
Comment on attachment 618967 [details] [diff] [review]
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler

This patch is actual implementation of D3E WheelEvent in ESM.

This patch add intDeltaX and intDeltaY to widget::WheelEvent. They are set line scroll amount by widget. Except the gesture handler for Windows, native event tells us when we should dispatch legacy line scroll event. When this isn't zero, PoseHandleEvent() dispatches legacy line scroll event.

And also, if intDeltaX and intDeltaY are never sets non-zero, the dispatcher sets isPixelOnlyDevice to true. Then, PixelDeltaAccumulator which is used by PreHandleEvent() accumulates pixel delta values for setting intDeltaX and intDeltaY. If the value becomes over 1 or less -1, it sets intDelta values automatically.

History and Zoom should be performed only when the intDelta value isn't 0.

Finally, DoScrollText() always calls nsIScrollableFrame::ScrollBy() with DEVICE_PIXELS. The amount is computed with the result of GetScrollAmount() and event's delta value and event's deltaMode value. By passing the "origin", it shouldn't break the new smooth scrolling.

I'll request other reviews after part.8's review is granted.

FYI: A lot of behavior which is implemented by this patch will be tested by part.10.
Comment 50 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-16 08:21:52 PDT
Comment on attachment 621863 [details] [diff] [review]
part.1 Add DOM3 WheelEvent


>@@ -623,16 +623,20 @@ NON_IDL_EVENT(draggesture,
> NON_IDL_EVENT(overflow,
>               NS_SCROLLPORT_OVERFLOW,
>               EventNameType_XUL,
>               NS_EVENT_NULL)
> NON_IDL_EVENT(underflow,
>               NS_SCROLLPORT_UNDERFLOW,
>               EventNameType_XUL,
>               NS_EVENT_NULL)
>+NON_IDL_EVENT(wheel,
>+              NS_WHEEL_WHEEL,
>+              EventNameType_XUL,
>+              NS_WHEEL_EVENT)
Ok for now, but there could be a followup to add this to idl and
html elements.


>@@ -829,16 +834,18 @@ nsEventDispatcher::CreateEvent(nsPresCon
>   // And if we didn't get an event, check the type argument.
> 
>   if (aEventType.LowerCaseEqualsLiteral("mouseevent") ||
>       aEventType.LowerCaseEqualsLiteral("mouseevents") ||
>       aEventType.LowerCaseEqualsLiteral("popupevents"))
>     return NS_NewDOMMouseEvent(aDOMEvent, aPresContext, nsnull);
>   if (aEventType.LowerCaseEqualsLiteral("mousescrollevents"))
>     return NS_NewDOMMouseScrollEvent(aDOMEvent, aPresContext, nsnull);
>+  if (aEventType.LowerCaseEqualsLiteral("wheelevent"))
>+    return NS_NewDOMWheelEvent(aDOMEvent, aPresContext, nsnull);
I think we don't need this.

>+  void initWheelEvent(in DOMString typeArg,
>+                      in boolean canBubbleArg,
>+                      in boolean cancelableArg,
>+                      in nsIDOMWindow viewArg,
>+                      in long detailArg,
>+                      in long screenXArg,
>+                      in long screenYArg,
>+                      in long clientXArg,
>+                      in long clientYArg,
>+                      in unsigned short buttonArg,
>+                      in nsIDOMEventTarget relatedTargetArg,
>+                      in DOMString modifiersListArg,
>+                      in double deltaXArg,
>+                      in double deltaYArg,
>+                      in double deltaZArg,
>+                      in unsigned long deltaMode);
Make this [noscript]. The spec doesn't have init* anymore.



Looks good, but could you update the test to not test initWheelEvent. I could look at them then.
Comment 51 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-17 01:52:56 PDT
Created attachment 624675 [details] [diff] [review]
part.1 Add DOM3 WheelEvent
Comment 52 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-17 01:53:33 PDT
Created attachment 624676 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests
Comment 53 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-17 01:54:06 PDT
Created attachment 624677 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 54 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-23 04:00:18 PDT
Comment on attachment 618960 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText()


>+
>+  /**
>+   * ComputeScrollTarget() returns a scrollable frame which should be
>+   * scrolled.
the scrollable frame

>+   *
>+   * @param aTargetFrame        The event target of wheel event.
of the wheel event

>+   * @param aEvent              The handling mouse wheel event.
of the wheel event





This patch could land separately, right?
Comment 55 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-23 04:21:05 PDT
Comment on attachment 618961 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events

>   /**
>    * ComputeScrollTarget() returns a scrollable frame which should be
>    * scrolled.
>    *
>    * @param aTargetFrame        The event target of wheel event.
>    * @param aEvent              The handling mouse wheel event.
>+   * @param aForDefaultAction   Whether this uses wheel transaction or not.
>+   *                            If true, returns the latest scrolled frame if
>+   *                            there is it.  Otherwise, the nearest ancestor
>+   *                            scrollable frame from aTargetFrame.
>    * @return                    The scrollable frame which should be scrolled.
>    */
aForDefaultAction is a bit odd name, but I couldn't come up with anything better.


>+  /**
>+   * GetScrollAmount() returns the scroll amount in app units of one line or
>+   * one page.  If the mouse scroll event scroll a page, returns the page width
scrolls
Comment 56 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-23 18:19:49 PDT
Created attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

Thank you, smaug.
Comment 57 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-23 18:22:03 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

Could you review dictionary_helper_gen.py part?
Comment 58 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-23 18:28:02 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

D3E Spec defines deltaX, deltaY and deltaZ as float. But if we do so, there are some issues, see comment 4 and latter comments (my example in comment 4 is wrong, replace "1.0" with "1/3" or something). Therefore, this patch uses double for them.
Comment 59 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-23 18:41:31 PDT
Created attachment 626664 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText()

(In reply to Olli Pettay [:smaug] from comment #54)
> Comment on attachment 618960 [details] [diff] [review]
> part.2 Separate code for deciding scroll target from
> nsEventStateManager::DoScrollText()
> 

> >+   * @param aEvent              The handling mouse wheel event.
> of the wheel event

?? I ignore this...

> This patch could land separately, right?

Yeah, but I think that we shouldn't do so actually. For the risk. I want to land all patches for this bug in early stage of 16.
Comment 60 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-23 19:00:13 PDT
Created attachment 626669 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events
Comment 61 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-24 01:41:03 PDT
Created attachment 626732 [details] [diff] [review]
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler
Comment 62 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-24 01:41:40 PDT
Created attachment 626733 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 63 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-24 01:42:15 PDT
Created attachment 626734 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa
Comment 64 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-24 01:42:59 PDT
Created attachment 626735 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event
Comment 65 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-05-25 12:06:32 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

Redirecting to Olli, since he will probably get to this before I can.
Comment 66 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-25 18:01:11 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

khuey:

He's done already :-)

I want you to confirm the dictionary part.
Comment 67 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-28 09:07:22 PDT
Comment on attachment 618962 [details] [diff] [review]
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs


>-nsEventStateManager::GetScrollLinesFor(nsMouseScrollEvent* aMouseEvent)
>-{
>-  NS_ASSERTION(!UseSystemScrollSettingFor(aMouseEvent),
>-    "GetScrollLinesFor() called when should use system settings");
>-  nsCAutoString prefName;
>-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
>-  prefName.Append(".numlines");
>-  return Preferences::GetInt(prefName.get());
>-}
>-
>-bool
>-nsEventStateManager::UseSystemScrollSettingFor(nsMouseScrollEvent* aMouseEvent)
>-{
>-  nsCAutoString prefName;
>-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
>-  prefName.Append(".sysnumlines");
>-  return Preferences::GetBool(prefName.get());
>-}
So what handles these cases with new prefs?
Comment 68 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-28 10:06:29 PDT
Comment on attachment 618964 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code


>-static PRUint32 gPixelScrollDeltaTimeout = 0;
>+PRInt32 nsEventStateManager::PixelDeltaAccumulator::sX = 0;
>+PRInt32 nsEventStateManager::PixelDeltaAccumulator::sY = 0;
>+TimeStamp nsEventStateManager::PixelDeltaAccumulator::sLastTime;

I was told that this kind of static initializers should be avoided
(TimeStamp has a ctor)
See for example bug 569629
I don't quite understand the details.

Could we perhaps keep PixelDeltaAccumulator in heap and create the object
in ESM ctor when first ESM is created.
Comment 69 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-28 10:09:31 PDT
Comment on attachment 618966 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM

I guess some other patch explains why this can be dropped.
Comment 70 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-05-28 10:28:11 PDT
Comment on attachment 626732 [details] [diff] [review]
part.8 Replace legacy mouse scroll event handlers with D3E wheel event handler


>+  // If the event don't scroll to both X and Y, we don't need to do here.
s/don't/doesn't/
we don't need to do here what? I guess anything

>-  bool isTrusted = (aEvent->flags & NS_EVENT_FLAG_TRUSTED) != 0;
>-  nsMouseScrollEvent event(isTrusted, NS_MOUSE_SCROLL, nsnull);
>+  nsMouseScrollEvent event(NS_IS_TRUSTED_EVENT(aEvent), NS_MOUSE_SCROLL,
>+                           aEvent->widget);
>+  if (*aStatus == nsEventStatus_eConsumeNoDefault) {
>+    event.flags |= NS_EVENT_FLAG_NO_DEFAULT;
>+  }
Why is this needed?

>-  bool isTrusted = (aEvent->flags & NS_EVENT_FLAG_TRUSTED) != 0;
>-  nsMouseScrollEvent event(isTrusted, NS_MOUSE_PIXEL_SCROLL, nsnull);
>+  nsMouseScrollEvent event(NS_IS_TRUSTED_EVENT(aEvent), NS_MOUSE_PIXEL_SCROLL,
>+                           aEvent->widget);
>+  if (*aStatus == nsEventStatus_eConsumeNoDefault) {
>+    event.flags |= NS_EVENT_FLAG_NO_DEFAULT;
>+  }
ditto

I rarely ask this, but could you split this to smaller pieces. This is hard to review.
Comment 71 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-28 20:55:18 PDT
(In reply to Olli Pettay [:smaug] from comment #67)
> Comment on attachment 618962 [details] [diff] [review]
> part.4 Add reverting delta value prefs instead of customizing number of
> scroll lines prefs
> 
> 
> >-nsEventStateManager::GetScrollLinesFor(nsMouseScrollEvent* aMouseEvent)
> >-{
> >-  NS_ASSERTION(!UseSystemScrollSettingFor(aMouseEvent),
> >-    "GetScrollLinesFor() called when should use system settings");
> >-  nsCAutoString prefName;
> >-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
> >-  prefName.Append(".numlines");
> >-  return Preferences::GetInt(prefName.get());
> >-}
> >-
> >-bool
> >-nsEventStateManager::UseSystemScrollSettingFor(nsMouseScrollEvent* aMouseEvent)
> >-{
> >-  nsCAutoString prefName;
> >-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
> >-  prefName.Append(".sysnumlines");
> >-  return Preferences::GetBool(prefName.get());
> >-}
> So what handles these cases with new prefs?

The new delta reverting prefs just switch the scroll direction, not change the scroll amount but the old prefs changed line scroll events to page scroll events. Therefore, the old prefs make some problems with acceleration and deciding its default action but the new ones don't so.

I think that for some users who want to customize scroll speed only on Fx, we should provide other prefs in widget level. E.g., on Windows, scroll amount is computed from ((native delta value) / 120 * (scroll amount per 120)). The native delta value can be various values if the mouse supports high-resolution scroll. So, current prefs, which replacing delta values with fixed values, don't make sense for high resolution scroll supporting devices. Similarly, on Mac, some devices cause line scroll, but the others cause pixel scroll. And the delta values are specified by OS directly and the value can be various. Although I don't have good idea for customizing the scrolling speed on Mac, but I think replacing delta values doesn't make sense.

Note that part.12 (Win) and part.13 (GTK) have new prefs for customizing scrolling speed.
Comment 72 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-28 21:01:25 PDT
(In reply to Olli Pettay [:smaug] from comment #69)
> Comment on attachment 618966 [details] [diff] [review]
> part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM
> 
> I guess some other patch explains why this can be dropped.

The event is used only by the widget for Windows. The event returns line height, page height and page width of the default action's scroll target. For supporting high resolution scrolling, the widget dispatches pixel scroll event whose delta value is computed with the result. However, new D3E WheelEvent supports high resolution line/page scroll by default (because the delta values are fraction). Therefore, the widget don't need to dispatch pixel wheel event after this bug. So, we don't need the event after this.
Comment 73 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-28 21:08:00 PDT
(In reply to Olli Pettay [:smaug] from comment #70)
> Comment on attachment 626732 [details] [diff] [review]
> part.8 Replace legacy mouse scroll event handlers with D3E wheel event
> handler

> I rarely ask this, but could you split this to smaller pieces. This is hard
> to review.

Hmm, okay, I'll separate them by purposes. However, some of separated patches could not make sense and be able to build.
Comment 74 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-30 01:58:52 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #71)
> (In reply to Olli Pettay [:smaug] from comment #67)
> > Comment on attachment 618962 [details] [diff] [review]
> > part.4 Add reverting delta value prefs instead of customizing number of
> > scroll lines prefs
> > 
> > 
> > >-nsEventStateManager::GetScrollLinesFor(nsMouseScrollEvent* aMouseEvent)
> > >-{
> > >-  NS_ASSERTION(!UseSystemScrollSettingFor(aMouseEvent),
> > >-    "GetScrollLinesFor() called when should use system settings");
> > >-  nsCAutoString prefName;
> > >-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
> > >-  prefName.Append(".numlines");
> > >-  return Preferences::GetInt(prefName.get());
> > >-}
> > >-
> > >-bool
> > >-nsEventStateManager::UseSystemScrollSettingFor(nsMouseScrollEvent* aMouseEvent)
> > >-{
> > >-  nsCAutoString prefName;
> > >-  GetBasePrefKeyForMouseWheel(aMouseEvent, prefName);
> > >-  prefName.Append(".sysnumlines");
> > >-  return Preferences::GetBool(prefName.get());
> > >-}
> > So what handles these cases with new prefs?
> 
> The new delta reverting prefs just switch the scroll direction, not change
> the scroll amount but the old prefs changed line scroll events to page
> scroll events. Therefore, the old prefs make some problems with acceleration
> and deciding its default action but the new ones don't so.
> 
> I think that for some users who want to customize scroll speed only on Fx,
> we should provide other prefs in widget level. E.g., on Windows, scroll
> amount is computed from ((native delta value) / 120 * (scroll amount per
> 120)). The native delta value can be various values if the mouse supports
> high-resolution scroll. So, current prefs, which replacing delta values with
> fixed values, don't make sense for high resolution scroll supporting
> devices. Similarly, on Mac, some devices cause line scroll, but the others
> cause pixel scroll. And the delta values are specified by OS directly and
> the value can be various. Although I don't have good idea for customizing
> the scrolling speed on Mac, but I think replacing delta values doesn't make
> sense.
> 
> Note that part.12 (Win) and part.13 (GTK) have new prefs for customizing
> scrolling speed.

And if user customized the prefs, nsMouseScrollEvent::customizedByUserPrefs would be true. Then, handlers can decide anything from the value like checking the result of UseSystemScrollSettingFor().
Comment 75 Johnny Stenback (:jst, jst@mozilla.com) 2012-05-30 09:35:05 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

sr=jst, and if this is needed for the next uplift, please go ahead and land, Kyle is on vacation this whole week, and he can look later if needed. I did look over the dictionary changes, and it looks good to me.
Comment 76 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-30 18:28:11 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #75)
> Comment on attachment 626661 [details] [diff] [review]
> part.1 Add DOM3 WheelEvent
> 
> sr=jst, and if this is needed for the next uplift, please go ahead and land,
> Kyle is on vacation this whole week, and he can look later if needed. I did
> look over the dictionary changes, and it looks good to me.

Thank you, but no problem. This bug should be fixed in early stage due to big change.
Comment 77 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-30 18:32:14 PDT
Comment on attachment 626661 [details] [diff] [review]
part.1 Add DOM3 WheelEvent

Canceling the dictionary part review due to jst confirmation. But you're welcome to marking r as minus if you found something wrong.
Comment 78 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-30 19:38:28 PDT
(In reply to Olli Pettay [:smaug] from comment #70)
> Comment on attachment 626732 [details] [diff] [review]
> part.8 Replace legacy mouse scroll event handlers with D3E wheel event
> handler

> >-  bool isTrusted = (aEvent->flags & NS_EVENT_FLAG_TRUSTED) != 0;
> >-  nsMouseScrollEvent event(isTrusted, NS_MOUSE_SCROLL, nsnull);
> >+  nsMouseScrollEvent event(NS_IS_TRUSTED_EVENT(aEvent), NS_MOUSE_SCROLL,
> >+                           aEvent->widget);
> >+  if (*aStatus == nsEventStatus_eConsumeNoDefault) {
> >+    event.flags |= NS_EVENT_FLAG_NO_DEFAULT;
> >+  }
> Why is this needed?

I checked current ESM behavior again. Then, the dispatchers are not set them. I think that current behavior is wrong. If an event in a series of events is consumed, other events should be consumed or not dispatched, right?

Currently, our keypress event is consumed at dispatch if keydown event was consumed. So, I think that legacy mouse scroll events should be same (for compatibility, we should dispatch but consume it).
Comment 79 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-30 19:57:26 PDT
So, I think that MozMousePixelScroll is default action of DOMMouseScroll. And both of them are default action of D3E WheelEvent.
Comment 80 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:17:27 PDT
Created attachment 628948 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code
Comment 81 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:19:29 PDT
Created attachment 628952 [details] [diff] [review]
part.8-1 Drop legacy mouse scroll events from IPC
Comment 82 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:20:36 PDT
Created attachment 628953 [details] [diff] [review]
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event
Comment 83 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:24:04 PDT
Created attachment 628955 [details] [diff] [review]
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent

LimitToOnePageScroll() is removed by this patch.

Same check will be implemented by part.8-4 in nsEventStateManager::DoScrollText().
Comment 84 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:27:41 PDT
Created attachment 628956 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll

This patch implements simple wheel event handler which only supports scroll for the default action.

There are two |#if 0|, they are not necessary for the simple event handling, but they cause bustage, so, temporarily disable them.
Comment 85 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:30:12 PDT
Created attachment 628959 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed

This makes legacy mouse events dispatched.

This patch still consume following pixel event if mouse scroll event is consumed. If you still think they're not necessary, I'll remove them.
Comment 86 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:33:32 PDT
Created attachment 628961 [details] [diff] [review]
part.8-6 Init intDeltaX and intDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device

Windows' gesture causes wheel events which cannot specify intDelta values because there is no information about lines. This patch supports such pixel scroll only devices. The intDelta values are initialized by the accumulator.
Comment 87 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:36:20 PDT
Created attachment 628963 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent

This patch implements moving history and zooming in/out for the default action of wheel event. If you have better idea of widget::WheelEvent::GetPreferredIntDelta(), let me know.
Comment 88 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:38:34 PDT
Created attachment 628964 [details] [diff] [review]
part.8-8 widget should be able to speicify the scroll type

Current implementation allows widget to specify scroll type which is, whether it allows smooth scrolling or not. This patch allows that.
Comment 89 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:39:20 PDT
Created attachment 628966 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaX in ESM
Comment 90 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-05-31 16:39:50 PDT
Created attachment 628969 [details] [diff] [review]
part.8-10 Remove the code handling legacy mouse events in layout
Comment 91 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 13:08:43 PDT
Comment on attachment 628948 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code

>+  PRInt32 numLines;
>+  if (isHorizontal) {
>+    mX += aEvent->delta;
>+    numLines =
>+      static_cast<PRInt32>(NS_round(static_cast<double>(mX) / pixelsPerLine));
>+    mX -= numLines * pixelsPerLine;
>+  } else {
>+    mY += aEvent->delta;
>+    numLines =
>+      static_cast<PRInt32>(NS_round(static_cast<double>(mY) / pixelsPerLine));
>+    mY -= numLines * pixelsPerLine;
>+  }
This is somewhat hard to follow. Some comment would be good.
Comment 92 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 13:28:30 PDT
Comment on attachment 628961 [details] [diff] [review]
part.8-6 Init intDeltaX and intDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device

intDelta is strange name. (It is added in some other patch.)
Perhaps lineDelta?
Comment 93 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 13:33:17 PDT
Comment on attachment 628966 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaX in ESM

this is about deltaZ, not deltaX
Comment 94 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 13:42:07 PDT
Comment on attachment 628963 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent

>+
>+  /**
>+   * Computes the action for the aEvent with prefs.  The result is
>+   * one of WheelAction.
>+   *
>+   * @param aUseSystemSettings    Set the result of UseSystemScrollSettingFor().
>+   */
>+  WheelAction ComputeWheelActionFor(mozilla::widget::WheelEvent* aEvent);
There is no param 'aUseSystemSettings'

>+
>+  /**
>+   * Gets the wheel action for the aMouseEvent ONLY with the pref.
>+   * When you actually do something for the event, probably you should use
>+   * ComputeWheelActionFor().
>+   */
>+  WheelAction GetWheelActionPrefFor(mozilla::widget::WheelEvent* aEvent);
> 
Could you somehow improve the comments of these two methods. It is hard
to understand the difference between them.
Comment 95 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 14:03:36 PDT
Comment on attachment 628955 [details] [diff] [review]
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent


>+struct DeltaValues {
{ should be in its own line


Good stuff :)
Comment 96 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-19 14:16:39 PDT
Comment on attachment 628959 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed


>+  /**
>+   * DeltaDirection is used for specifying whether the called method should
>+   * handle vertical delta or horizontal delta.
>+   * This is clearer than using bool.
>+   */
>+  enum DeltaDirection {
{ should be on its own line


>+  // If widget sets intDelta, nsEventStateManager will dispatch NS_MOUSE_SCROLL
>+  // event for compatibility.
>+  PRInt32 intDeltaX;
>+  PRInt32 intDeltaY;
These are oddly named. lineDeltaX/Y perhaps
Also, shouldn't ESM always try to dispatch DOMMouseScroll. That is what all the web pages expect
from Gecko now. Or do all the widget implementations set these deltas?
Comment 97 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-19 20:25:45 PDT
(In reply to Olli Pettay [:smaug] from comment #96)
> Comment on attachment 628959 [details] [diff] [review]
> part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the
> wheel event isn't consumed
> >+  // If widget sets intDelta, nsEventStateManager will dispatch NS_MOUSE_SCROLL
> >+  // event for compatibility.
> >+  PRInt32 intDeltaX;
> >+  PRInt32 intDeltaY;
> These are oddly named. lineDeltaX/Y perhaps

"line" doesn't sounds good for page scroll but it's ok for me. Do you think it's not problem?

> Also, shouldn't ESM always try to dispatch DOMMouseScroll. That is what all
> the web pages expect from Gecko now.

Did you mean that we should dispatch DOMMouseScroll even if its detail value is zero? If so, I don't think so. Both widgets of Mac and Windows don't dispatch NS_MOUSE_SCROLL event if detail value is less than 0.

> Or do all the widget implementations set these deltas?

Almost yes.

On Mac, native mouse wheel event has both delta values for lines and pixels. If lines are not zero, part.14 sets the values.

On Windows, native mouse wheel messages only have wheel turned distance as the delta value. Actual scroll amount for 120 of the delta values are defined by registry. Our widget for Windows also accumulates native delta values and when it becomes over one line (or page), part.12 sets the integer values.

But on Windows, native gesture event handler only know how much pixels should be scrolled current gesture. In this case, the handler always sets 0 for the values. And delta accumulator in ESM will compute the values from scroll target's line height.

On the other platforms, deltaX/deltaY and these values are always same because they don't have high-resolution scroll.
Comment 98 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:00:51 PDT
Created attachment 634776 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)
Comment 99 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:01:34 PDT
Created attachment 634777 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (r=smaug)
Comment 100 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:02:52 PDT
Created attachment 634778 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (r=smaug)
Comment 101 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:07:24 PDT
Created attachment 634779 [details] [diff] [review]
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs

See comment 71 and comment 74 for my replies to your first review.
Comment 102 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:08:08 PDT
Created attachment 634780 [details] [diff] [review]
part.5 Don't separate wheel action prefs by direction (r=smaug)
Comment 103 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:09:01 PDT
Created attachment 634781 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code (r=smaug)
Comment 104 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:09:41 PDT
Created attachment 634783 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug)
Comment 105 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:10:22 PDT
Created attachment 634784 [details] [diff] [review]
part.8-1 Drop legacy mouse scroll events from IPC (r=smaug)
Comment 106 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:10:58 PDT
Created attachment 634785 [details] [diff] [review]
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug)
Comment 107 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:11:35 PDT
Created attachment 634786 [details] [diff] [review]
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug)
Comment 108 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:12:10 PDT
Created attachment 634787 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)
Comment 109 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:13:37 PDT
Created attachment 634788 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed

See comment 97.
Comment 110 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:14:37 PDT
Created attachment 634790 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device (r=smaug)
Comment 111 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:18:06 PDT
Created attachment 634792 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent

I removed |GetWheelActionPrefFor()| because the user is only |ComputeWheelActionFor()|. I think that it is better to prevent confusion than the old code.
Comment 112 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:19:12 PDT
Created attachment 634793 [details] [diff] [review]
part.8-8 widget should be able to speicify the scroll type (r=smaug)
Comment 113 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:19:57 PDT
Created attachment 634794 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 114 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:20:48 PDT
Created attachment 634795 [details] [diff] [review]
part.8-10 Remove the code handling legacy mouse events in layout (r=smaug)
Comment 115 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:21:58 PDT
Created attachment 634796 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests
Comment 116 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:23:06 PDT
Created attachment 634797 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 117 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:23:42 PDT
Created attachment 634798 [details] [diff] [review]
part.11 Fix new test failures
Comment 118 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:24:22 PDT
Created attachment 634799 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows
Comment 119 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:24:53 PDT
Created attachment 634800 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK
Comment 120 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:25:33 PDT
Created attachment 634801 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa
Comment 121 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:26:14 PDT
Created attachment 634802 [details] [diff] [review]
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt
Comment 122 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:26:56 PDT
Created attachment 634803 [details] [diff] [review]
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2
Comment 123 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:27:33 PDT
Created attachment 634804 [details] [diff] [review]
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils
Comment 124 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:29:03 PDT
Created attachment 634805 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event
Comment 125 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:31:50 PDT
Comment on attachment 634804 [details] [diff] [review]
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils

I'm not sure who are using this method. I think smaug's review is enough for such change even if there is another owner.
Comment 126 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:34:08 PDT
Comment on attachment 634801 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa

By this patch, there is no user of nsMouseScrollEvent::kIsMomentum.

# Of course, I'll request to review to cocoa widget reviewer later.
Comment 127 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:38:19 PDT
Comment on attachment 634799 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows

By this patch, we can completely remove NS_QUERY_SCROLL_TARGET_INFO event.

And also we can remove nsMouseScrollEvent::kNoLines, nsMouseScrollEvent::kNoDefer, nsMouseScrollEvent::kAllowSmoothScroll and nsMouseScrollEvent::kFromLines.
Comment 128 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 00:40:46 PDT
I'll request to Smaug to review for part.10 and part.11 after part.9. But you're welcome if you review them before I'll request them.
Comment 129 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-20 00:57:13 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #97)
> > These are oddly named. lineDeltaX/Y perhaps
> 
> "line" doesn't sounds good for page scroll but it's ok for me. Do you think
> it's not problem?
>
Oh, right, it can be page scroll too. then lineDelta isn't good. but int* is just very vague.


> > Also, shouldn't ESM always try to dispatch DOMMouseScroll. That is what all
> > the web pages expect from Gecko now.
> 
> Did you mean that we should dispatch DOMMouseScroll even if its detail value
> is zero? If so, I don't think so. Both widgets of Mac and Windows don't
> dispatch NS_MOUSE_SCROLL event if detail value is less than 0.

Ah, right. No DOMMouseScroll if there isn't really anything to report.
Comment 130 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 01:33:41 PDT
(In reply to Olli Pettay [:smaug] from comment #129)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #97)
> > > These are oddly named. lineDeltaX/Y perhaps
> > 
> > "line" doesn't sounds good for page scroll but it's ok for me. Do you think
> > it's not problem?
> >
> Oh, right, it can be page scroll too. then lineDelta isn't good. but int* is
> just very vague.

Hmm...

For the purpose, legacyDeltaX/legacyDeltaY might be better. But I think the developers of widgets cannot understand the meaning of them by the names...
Comment 131 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-06-20 01:34:47 PDT
Maybe lineOrPageDeltaX/Y ? That is a bit ugly but should be reasonable clear.
Comment 132 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 01:42:11 PDT
(In reply to Olli Pettay [:smaug] from comment #131)
> Maybe lineOrPageDeltaX/Y ? That is a bit ugly but should be reasonable clear.

Okay, I'll replace lineDelta* with them.
Comment 133 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:26:38 PDT
Created attachment 634878 [details] [diff] [review]
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug)
Comment 134 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:27:21 PDT
Created attachment 634879 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed
Comment 135 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:28:26 PDT
Created attachment 634880 [details] [diff] [review]
part.8-6 Init lineOrPageDeltaX and lineOrPageDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device (r=smaug)
Comment 136 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:29:08 PDT
Created attachment 634881 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent
Comment 137 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:29:46 PDT
Created attachment 634882 [details] [diff] [review]
part.8-8 widget should be able to speicify the scroll type (r=smaug)
Comment 138 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:30:29 PDT
Created attachment 634883 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 139 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:31:10 PDT
Created attachment 634884 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests
Comment 140 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:31:42 PDT
Created attachment 634885 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 141 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:32:16 PDT
Created attachment 634887 [details] [diff] [review]
part.11 Fix new test failures
Comment 142 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:33:06 PDT
Created attachment 634888 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows
Comment 143 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:33:41 PDT
Created attachment 634889 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK
Comment 144 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:34:19 PDT
Created attachment 634890 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa
Comment 145 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:34:55 PDT
Created attachment 634891 [details] [diff] [review]
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt
Comment 146 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:35:38 PDT
Created attachment 634892 [details] [diff] [review]
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2
Comment 147 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-20 06:36:14 PDT
Created attachment 634893 [details] [diff] [review]
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils
Comment 148 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-06-28 03:18:04 PDT
I filed a spec bug for the type of delta attributes:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=17628
Comment 149 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:23:56 PDT
Comment on attachment 634779 [details] [diff] [review]
part.4 Add reverting delta value prefs instead of customizing number of scroll lines prefs

So could you explain why we have
mousewheel.withaltkey.action, but the new stuff is 
mousewheel.with_alt.*

Also, can we remove .numlines without adding something similar back?
I thought people use that pretty often.
Perhaps we could have something like mousewheel.fookey.accelerationLevel.
It would be 1 by default. -1 would reverse the direction, 2 would double
the scrolling speed (-2 opposite direction)
Comment 150 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:29:50 PDT
Comment on attachment 634805 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event


>+#ifndef nsMouseScrollEvent_h__
>+#define nsMouseScrollEvent_h__

I don't like moving nsMouseScrollEvent to its own .h
nsMutationEvent.h is an exception, and it makes things just harder than they should be.
Comment 151 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:39:36 PDT
Comment on attachment 634879 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the wheel event isn't consumed

Why this way. I was thinking we should dispatch first the legacy events and then
if those aren't preventDefault'ed, dispatch wheel event.
That way for example addons could listen for wheel events only and handle them
knowing that content hasn't consumed the old legacy events.
Comment 152 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:45:06 PDT
Comment on attachment 634881 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent


>+  // Momentum events shouldn't run special actions.
>+  if (aEvent->isMomentum) {
Should this be 
!aEvent->isMomentum
Comment 153 Daniel Friesen 2012-07-04 10:53:15 PDT
(In reply to Olli Pettay [:smaug] from comment #151)
> Comment on attachment 634879 [details] [diff] [review]
> part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the
> wheel event isn't consumed
> 
> Why this way. I was thinking we should dispatch first the legacy events and
> then
> if those aren't preventDefault'ed, dispatch wheel event.
> That way for example addons could listen for wheel events only and handle
> them
> knowing that content hasn't consumed the old legacy events.

Most tactics I've seen for dealing with multiple events of the same type (legacy and new better events) involve registering handlers for all the events. And when the newer events are fired flipping a boolean that tells the code to ignore the legacy events.

If we dispatch the legacy events first and handle preventDefault that way we risk making code already written to handle wheel events react twice to the same scroll making things buggy. And if the app is coded to use a e.type == string instead of multiple booleans (easier way to handle the fact we now have 3 possible events) we risk making it only use legacy events instead of taking advantage of modern events.
Comment 154 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:55:46 PDT
Comment on attachment 634890 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa

I think some OSX widget peer should look at this too.
Comment 155 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 10:59:47 PDT
(In reply to Daniel Friesen from comment #153)
> Most tactics I've seen for dealing with multiple events of the same type
> (legacy and new better events)
Just curious which case have you in mind? I don't recall any other case when legacy event is being
deprecated this way.

> involve registering handlers for all the
> events.
Old code won't do that. And new code should just use the new events

 And when the newer events are fired flipping a boolean that tells
> the code to ignore the legacy events.
> 
> If we dispatch the legacy events first and handle preventDefault that way we
> risk making code already written to handle wheel events react twice to the
> same scroll making things buggy.
How so? If you handle the old events, you call preventDefault() and the listener for new wheel event doesn't get
called at all.
Comment 156 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 11:06:36 PDT
Comment on attachment 634888 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows

Jimm or bbondy or felipe should review this too.
Comment 157 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 11:26:44 PDT
And still to clarify, web page doesn't need to listen for legacy events if it does
the usual feature testing first.
if ("WheelEvent" in window) {
  // new event handling...
} else {
  // legacy
}
Comment 158 Daniel Friesen 2012-07-04 12:09:07 PDT
(In reply to Olli Pettay [:smaug] from comment #157)
> And still to clarify, web page doesn't need to listen for legacy events if
> it does
> the usual feature testing first.
> if ("WheelEvent" in window) {
>   // new event handling...
> } else {
>   // legacy
> }

I was worried about this kind of testing not working. Historically this has not been a very reliable way of testing for event existence since browsers are not consistent in whether they make this available or not. Notably Firefox has not placed these classes on window in the past. I guess this works in IE9 so I don't need to worry about that.

But doing some testing turned up a different issue with this plan. For some reason current versions of chrome have a window.WheelEvent present, but document.addEventListener('wheel', ..., false); does not fire any wheel events. So this kind of testing could lead to a site not functioning in Chrome.
Comment 159 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 12:13:52 PDT
I hope you have filed a bug on Chrome ;)
Comment 160 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 12:20:55 PDT
Hmm, in which way does IE dispatch wheel and mousewheel. We should probably use the same
order for wheel and DOMMouseScroll/MozPixelScroll.
Comment 161 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-04 13:12:04 PDT
...yet dispatching wheel first would effectively force chrome/addons use the legacy events.

One approach, if we need to dispatch wheel first, would be to dispatch legacy events
using nsDispatchingCallback. Then chrome could use system event group listeners for wheel.
Effectively events would be
wheel (in default group), legacy events (in default group), legacy events (in system group),  wheel (in system group)
Comment 162 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-04 17:36:33 PDT
(In reply to Olli Pettay [:smaug] from comment #149)
> Comment on attachment 634779 [details] [diff] [review]
> part.4 Add reverting delta value prefs instead of customizing number of
> scroll lines prefs
> 
> So could you explain why we have
> mousewheel.withaltkey.action, but the new stuff is 
> mousewheel.with_alt.*

The reasons for changing the naming rules of the prefs are:

1. "withnokey" doesn't make sense if we use them for two or modifier keys are pressed. "default" is clearer for such purpose.

2. by changing the naming rules, we don't worry about the conflict between older prefs and newer prefs. The older prefs are kept in user pref if they are customized. So, it would break future change if the developer uses same name accidentally.

3. and the change notices users who want to customize wheel behavior that the all prefs are reimplemented.

4. and users can keep the each setting for both in normal build and ESR.

> Also, can we remove .numlines without adding something similar back?

First, I think current approach is wrong at least after we support pixel scroll events because if we multiply the delta value by something come from prefs, it would work only when delta mode line and page. For example:

event1: pixel=3, line=1  ---->  pixel=6, line=2
event2: pixel=3, line=0         pixel=6, line=0
event3: pixel=3, line=0         pixel=6, line=0
event4: pixel=3, line=1         pixel=6, line=2
event5: pixel=3, line=0         pixel=6, line=0
event6: pixel=3, line=0         pixel=6, line=0

So, normal scrolling must work fine if the delta mode is pixel. However, I think that event1, event3, event4 and event6 should have line=1.

I was thinking that we should overwrite the delta values in widget level due to the native event differences. However, I still don't have an idea for sorting out this issue on MacOS X. If you have the idea and it can be implemented in ESM, we can implement the acceleration pref in the ESM.

(In reply to Olli Pettay [:smaug] from comment #150)
> Comment on attachment 634805 [details] [diff] [review]
> part.18 Clean up legacy mouse scroll event
> 
> 
> >+#ifndef nsMouseScrollEvent_h__
> >+#define nsMouseScrollEvent_h__
> 
> I don't like moving nsMouseScrollEvent to its own .h
> nsMutationEvent.h is an exception, and it makes things just harder than they
> should be.

Do you think that we should keep the event in nsGUIEvent.h? Then, I think that widget implementers might be confused by the difference between nsMouseScrollEvent and widget::WheelEvent. nsMouseScrollEvent is only used for the internal event of the legacy DOM events. So, I don't think that they should be defined in widget/.

(In reply to Olli Pettay [:smaug] from comment #151)
> Comment on attachment 634879 [details] [diff] [review]
> part.8-5 Dispatch legacy mouse scroll events from PostHandleEvent() if the
> wheel event isn't consumed
> 
> Why this way. I was thinking we should dispatch first the legacy events and
> then
> if those aren't preventDefault'ed, dispatch wheel event.
> That way for example addons could listen for wheel events only and handle
> them
> knowing that content hasn't consumed the old legacy events.

IE9 dispatches new "wheel" event, first. And then, dispatches legacy "mousewheel" events.

I think that this event order does make sense for web developers. Web developers may want to use "wheel" event for newer browsers. In such case, if we would dispatch legacy events first, they cannot know if wheel event would be fired, without UA check. Then, the path for older browsers would run for new Gecko browsers.

(In reply to Daniel Friesen from comment #153)
> Most tactics I've seen for dealing with multiple events of the same type
> (legacy and new better events) involve registering handlers for all the
> events. And when the newer events are fired flipping a boolean that tells
> the code to ignore the legacy events.

That's interesting, why don't you use preventDefault() for the first event if the first event is the most preferred event? Such approach doesn't work with other browsers?

> If we dispatch the legacy events first and handle preventDefault that way we
> risk making code already written to handle wheel events react twice to the
> same scroll making things buggy.

I think that it doesn't depend on the event order.

> And if the app is coded to use a e.type ==
> string instead of multiple booleans (easier way to handle the fact we now
> have 3 possible events) we risk making it only use legacy events instead of
> taking advantage of modern events.

Yeah, I assumed such implementations for deciding the order.

(In reply to Olli Pettay [:smaug] from comment #155)
> > involve registering handlers for all the
> > events.
> Old code won't do that. And new code should just use the new events
> 
>  And when the newer events are fired flipping a boolean that tells

That means such application ignores the DOM event set of first wheel event? At handling first DOM event, the application don't know if there is next event.

> > If we dispatch the legacy events first and handle preventDefault that way we
> > risk making code already written to handle wheel events react twice to the
> > same scroll making things buggy.
> How so? If you handle the old events, you call preventDefault() and the
> listener for new wheel event doesn't get
> called at all.

If the handler for legacy events have some limitations due to the difference of the event structure, our user may loose the better UX in such web application.

(In reply to Olli Pettay [:smaug] from comment #161)
> ...yet dispatching wheel first would effectively force chrome/addons use the
> legacy events.

I'm not sure what you meant here.

> One approach, if we need to dispatch wheel first, would be to dispatch
> legacy events
> using nsDispatchingCallback. Then chrome could use system event group
> listeners for wheel.
> Effectively events would be
> wheel (in default group), legacy events (in default group), legacy events
> (in system group),  wheel (in system group)

Hmm, I don't understand why we need to dispatch in the order.

(In reply to Olli Pettay [:smaug] from comment #157)
> And still to clarify, web page doesn't need to listen for legacy events if
> it does
> the usual feature testing first.
> if ("WheelEvent" in window) {
>   // new event handling...
> } else {
>   // legacy
> }

Awesome! I didn't have the idea.
Comment 163 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:14:00 PDT
(In reply to Olli Pettay [:smaug] from comment #152)
> Comment on attachment 634881 [details] [diff] [review]
> part.8-7 Implement the other default actions of WheelEvent
> 
> 
> >+  // Momentum events shouldn't run special actions.
> >+  if (aEvent->isMomentum) {
> Should this be 
> !aEvent->isMomentum

Why is that? If the event is caused by momentum scroll, it should cause only scroll or nothing.
Comment 164 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:16:56 PDT
Created attachment 639240 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)
Comment 165 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:19:06 PDT
Created attachment 639241 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (r=smaug)
Comment 166 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:20:19 PDT
Created attachment 639242 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug)
Comment 167 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:21:35 PDT
Created attachment 639243 [details] [diff] [review]
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2
Comment 168 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 00:22:41 PDT
Created attachment 639244 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event

# Just updated for the latest m-c.
Comment 169 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 01:28:18 PDT
I'm still thinking about the new pref design.

First, replacing delta values with fixed values doesn't make sense. That way kills some events which are caused by turning mouse wheel faster. And also it kills high resolution scrolling too.

These facts mean that the pref values should be multiplied by delta values.

Then, the values should be float, I think. It should be x100 values. Nobody need such resolution, but we're usually using x100 for float values in int prefs.

If the delta mode is pixel and the pref values are not 100 or -100, the delta accumulator should compute the lineOrPageDelta and replace the native event's value.

By this approach, it may be possible to implement in XP level.

Perhaps, the pref name should be .deltaMultiplier(X|Y|X)?
Comment 170 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-05 02:05:07 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #162)
 
> > So could you explain why we have
> > mousewheel.withaltkey.action, but the new stuff is 
> > mousewheel.with_alt.*
> 
> The reasons for changing the naming rules of the prefs are:
> 
> 1. "withnokey" doesn't make sense if we use them for two or modifier keys
> are pressed. "default" is clearer for such purpose.
> 
> 2. by changing the naming rules, we don't worry about the conflict between
> older prefs and newer prefs. The older prefs are kept in user pref if they
> are customized. So, it would break future change if the developer uses same
> name accidentally.
But mousewheel.withaltkey.reverse_deltaX wouldn't conflict with old prefs.
I'd just like to have mousewheel.withXXX in one way, not in two different ways.

> First, I think current approach is wrong at least after we support pixel
> scroll events because if we multiply the delta value by something come from
> prefs, it would work only when delta mode line and page.
Right. But it could work for line scrolling. I just don't want to force people, who have got used to
fast scrolling to use slower scrolling.


> Do you think that we should keep the event in nsGUIEvent.h? 
Yes.

> Then, I think
> that widget implementers might be confused by the difference between
> nsMouseScrollEvent and widget::WheelEvent. nsMouseScrollEvent is only used
> for the internal event of the legacy DOM events. So, I don't think that they
> should be defined in widget/.
nsGUIEvent contains lots of DOM only stuff.
(We could and should get rid of those ns*Event structs but not in this bug.)


> IE9 dispatches new "wheel" event, first. And then, dispatches legacy
> "mousewheel" events.
Ok, then I think we need to dispatch legacy events using the dispatching callback so that
addons/chrome can still use wheel event (in system group)


> (In reply to Olli Pettay [:smaug] from comment #161)
> > ...yet dispatching wheel first would effectively force chrome/addons use the
> > legacy events.
> 
> I'm not sure what you meant here.
The problem is that chrome/addons need to know whether preventDefault was called.
So, if they use wheel event and it is dispatched before legacy events, chrome/addons can't know
whether preventDefault was called on the legacy events.



> 
> > One approach, if we need to dispatch wheel first, would be to dispatch
> > legacy events
> > using nsDispatchingCallback. Then chrome could use system event group
> > listeners for wheel.
> > Effectively events would be
> > wheel (in default group), legacy events (in default group), legacy events
> > (in system group),  wheel (in system group)
> 
> Hmm, I don't understand why we need to dispatch in the order.
Because of the problem to detect event.defaultPrevented.
Comment 171 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-05 02:06:29 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #169)
> Perhaps, the pref name should be .deltaMultiplier(X|Y|X)?
Sounds ok.
Comment 172 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 02:42:43 PDT
Ok, then, I'll rewrite the patches. Thanks!
Comment 173 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 02:45:34 PDT
(In reply to Olli Pettay [:smaug] from comment #170)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #162)
>  
> > > So could you explain why we have
> > > mousewheel.withaltkey.action, but the new stuff is 
> > > mousewheel.with_alt.*
> > 
> > The reasons for changing the naming rules of the prefs are:
> > 
> > 1. "withnokey" doesn't make sense if we use them for two or modifier keys
> > are pressed. "default" is clearer for such purpose.
> > 
> > 2. by changing the naming rules, we don't worry about the conflict between
> > older prefs and newer prefs. The older prefs are kept in user pref if they
> > are customized. So, it would break future change if the developer uses same
> > name accidentally.
> But mousewheel.withaltkey.reverse_deltaX wouldn't conflict with old prefs.
> I'd just like to have mousewheel.withXXX in one way, not in two different
> ways.

Er, but the ".action" would conflict. Should we rename it to ".default_action"?
Comment 174 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-05 02:47:31 PDT
Hmm, which patch is adding mousewheel.with_alt.action ?
(So many patches here ;) )
Comment 175 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 03:20:55 PDT
(In reply to Olli Pettay [:smaug] from comment #174)
> Hmm, which patch is adding mousewheel.with_alt.action ?
> (So many patches here ;) )

See part.5 that removes the .horizontal and changes the meaning of the value.
https://bugzilla.mozilla.org/attachment.cgi?id=634780&action=diff
Comment 176 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-05 03:23:58 PDT
Ah, right. So after that change all the prefs are in form mousewheel.with_foo.x ?
That is good, and sorry that I missed it.
Comment 177 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 03:52:16 PDT
okay, no problem, the count of patches are too many for me too :-(
Comment 178 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 23:50:21 PDT
Created attachment 639590 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)

Updated for bug 769190.
Comment 179 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-05 23:51:45 PDT
Created attachment 639591 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_*

not tested yet.
Comment 180 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 00:03:50 PDT
Created attachment 639593 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs
Comment 181 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 00:12:44 PDT
Created attachment 639597 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code (r=smaug)
Comment 182 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 00:23:33 PDT
Created attachment 639600 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug)
Comment 183 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 00:34:42 PDT
Comment on attachment 634787 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)

Er, 8-4 doesn't need to be reviewed again.
Comment 184 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 01:03:46 PDT
Created attachment 639604 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)
Comment 185 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 01:28:09 PDT
Created attachment 639608 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed

not tested yet.
Comment 186 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 03:17:34 PDT
Created attachment 639614 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs

not tested yet.
Comment 187 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 04:10:03 PDT
Created attachment 639622 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent
Comment 188 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 06:08:41 PDT
Created attachment 639639 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 189 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-06 06:57:10 PDT
Created attachment 639650 [details] [diff] [review]
part.8-11 Cancel applying delta multipliers from overflowDelta
Comment 190 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:27:17 PDT
Created attachment 640937 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)
Comment 191 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:29:14 PDT
Comment on attachment 639591 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_*

We should support only faster setting (i.e., >= 1.0 or <= -1.0) for making simpler implementation for now.
Comment 192 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:31:01 PDT
Comment on attachment 639593 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs

Using nsEventStateManager::WheelPrefs for managing action prefs. The actual behavior isn't changed from the previous (you reviewed) patch.
Comment 193 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:33:22 PDT
Created attachment 640938 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)

Just adjusting the reviewed patch to the new patches.
Comment 194 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:34:59 PDT
Created attachment 640939 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed

The event order would be tested by the tests in part.10.
Comment 195 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:40:05 PDT
Created attachment 640943 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs

This patch computes the lineOrPageDelta values when one or more of the multiplier settings is changed. Note that we shouldn't ignore the settings for the delta values because it may cause unexpected behavior. We should always compute both accumulated delta values.
Comment 196 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:43:47 PDT
Created attachment 640945 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent

I still think that you misunderstood at previous review (comment 152). This patch is easier to read than the previous patch (by the WheelPrefs class). Could you review this again?
Comment 197 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:45:07 PDT
Created attachment 640946 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 198 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:47:28 PDT
Created attachment 640947 [details] [diff] [review]
part.8-11 Cancel applying delta multipliers from overflowDelta

I think that we need to cancel the inflation of overflowDelta which is caused by multiplier prefs.
Comment 199 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:53:55 PDT
Created attachment 640950 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler

I gave up to fix test_bug350471.xul. But other new tests covers same things. So, we can just remove it.
Comment 200 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:55:10 PDT
Created attachment 640951 [details] [diff] [review]
part.11 Fix new test failures
Comment 201 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:56:55 PDT
Created attachment 640954 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (r=smaug)

I'll request to review to jimm after other requested reviews.
Comment 202 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:57:52 PDT
Created attachment 640956 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK
Comment 203 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-10 23:59:08 PDT
Created attachment 640957 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event
Comment 204 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-11 08:59:17 PDT
Comment on attachment 640957 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event


>+/**
>+ * nsMouseScrollEvent is used for legacy DOM mouse scroll events, i.e.,
>+ * DOMMouseScroll and MozMousePixelScroll event.  These events are NOT hanbled
>+ * by ESM even if widget dispatches them.  Use new widget::WheelEvent instead.
s/ESM/nsEventStateManager/ That would be more clear for someone not too familiar with
event handling.
Comment 205 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-11 16:50:57 PDT
test build:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-b617e4e53fea/
Comment 206 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-18 02:14:27 PDT
Created attachment 643298 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_*

Including the fix for bug 310003.
Comment 207 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-18 02:15:33 PDT
Created attachment 643299 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs
Comment 208 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-18 02:16:17 PDT
Created attachment 643300 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 209 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-19 20:20:54 PDT
Smaug, could you review part.4 and part.5 ASAP? Then, the widget patches shouldn't be affected by other patches for ESM. So, I can request reviews to each widget reviewer.
Comment 210 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-20 03:33:24 PDT
Sorry about the delay. I'll review all the patches this weekend, starting from
part 4 and 5 today.
Comment 211 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-20 15:06:32 PDT
Argh, looks like tomorrow.
Comment 212 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-20 16:20:03 PDT
No problem, don't mind. I think that we need a week or more for reviews by widget people, so, I'd like you to review only the 4 and the 5 for now if you don't have much time (and I know that).
Comment 213 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-23 14:12:56 PDT
Comment on attachment 643298 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_*

>+    enum Index
>+    {
>+      INDEX_DEFAULT,
perhaps = 0,
Comment 214 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-07-23 14:27:24 PDT
Comment on attachment 643299 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs


>+
>+  NS_ERROR("Gets new wheel action pref value but it's not implemented yet.");

Perhaps just NS_WARNING
and something like
"Unsupported wheel action pref value!")

>+  nsCAutoString prefNameAction(basePrefName);
>+  prefNameAction.AppendLiteral("action");
>+  mActions[aIndex] =
>+    static_cast<Action>(Preferences::GetInt(prefNameAction.get(),
>+                                            ACTION_SCROLL));
>+  if (mActions[aIndex] < ACTION_NONE || mActions[aIndex] > ACTION_LAST) {
>+    mActions[aIndex] = ACTION_SCROLL;
Should we warn (NS_WARNING) in this case?
>+    enum Action
>+    {
>+      ACTION_NONE,
= 0
Comment 215 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 18:21:35 PDT
Thank you, I'll update the patches soon.
Comment 216 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 18:38:23 PDT
Comment on attachment 640954 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (r=smaug)

D3E WheelEvent allows high resolution line or page scroll. Therefore, our mouse wheel handler for Windows becomes very simple!

1. We don't need to dispatch pixel scroll event. Therefore, we can remove the dispatcher code.

2. We don't need to get scroll target's information due to #1. So, we can remove the dispatcher of NS_QUERY_SCROLL_TARGET_INFO (MouseScrollHandler::GetScrollTargetInfo()).

And also, a wheel event can handle both X and Y direction scrolling. Therefore, nsWinGesture::PanDeltaToPixelScrollX() and nsWinGesture::PanDeltaToPixelScrollY() are merged to a method.

For compatibility, nsEventStateManager needs to dispatch DOMMouseScroll event. WheelEvent::lineOrPageDelta(X|Y) tell the timing and amount. Therefore, we're still computing the accumulated delta in MouseScrollHandler.

On the other hand, we cannot set lineOrPageDelta(X|Y) in nsWinGesture because there is no information for the line height. Therefore, it should be computed in nsEventStateManager. If WheelEvent::isPixelOnlyDevice is true, nsEventStateManager will do it. Therefore, only nsWinGesture sets true to isPixelOnlyDevice.

Note that the XP part has been reviewed by smaug already, so, you don't need to review the part if you don't familiar with it.
Comment 217 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 18:42:43 PDT
Comment on attachment 640956 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK

On Linux, native wheel event doesn't support high resolution scrolling. Therefore, the widget for Linux only sets same delta value to delta(X|Y) and lineOrPageDelta(X|Y).
Comment 218 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 18:44:29 PDT
Comment on attachment 634891 [details] [diff] [review]
part.15 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Qt

Same as GTK.
Comment 219 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 18:51:29 PDT
Comment on attachment 639243 [details] [diff] [review]
part.16 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on OS/2

From the change log of nsWindow for OS/2, I'm requesting the review to Dave Yeo. But if you don't familiar with this code, let me know.

I have never built this patch actually.

Looks like the native wheel event of OS/2 doesn't support high resolution scroll, so, similar to Linux, we need to set same delta value to delta(X|Y) and lineOrPageDelta(X|Y).
Comment 220 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-23 19:05:25 PDT
Comment on attachment 639242 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug)

D3E WheelEvent's delta values can have the amount in either lines or pixels. So, we don't need to dispatch both line and pixel events. We need to set just WheelEvent::deltaMode to DOM_DELTA_LINE or DOM_DELTA_PIXEL.

And also, a D3E WheelEvent can have both X/Y delta values (i.e., supports oblique scroll). Therefore, the dispatcher sets both values at once.

WheelEvent::delta(X|Y) are the delta value which indicates the amount of scroll. On the other hand, lineOrPageDelta(X|Y) are used for dispatching legacy DOMMouseScroll events in nsEventStateManager. They must be set when we have dispatched NS_MOUSE_SCROLL event.

I've never tested the carbon event's part because I don't know which plugin supports mouse wheel scrolling on Mac. If you know it, I'll test it.

And note that the XP part has been reviewed by smaug already, so, you don't need to check it.
Comment 221 Karl Tomlinson (ni?:karlt) 2012-07-23 19:58:16 PDT
Comment on attachment 640956 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK

>+    if (!wheelEvent.deltaX && !wheelEvent.deltaY) {
>+        return;
>+    }

This shouldn't happen so no need to have this code in release builds.
Make this an assertion if you like.

>@@ -2747,30 +2762,34 @@ nsWindow::OnButtonPressEvent(GtkWidget *
>         break;
>     case 3:
>         domButton = nsMouseEvent::eRightButton;
>         break;
>     // These are mapped to horizontal scroll
>     case 6:
>     case 7:

Cases 6 and 7 shouldn't be reached.  They look like remnants from an ancient
GTK version (pre GTK+ 2.10 at least), so feel free to remove that code.
Comment 222 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:36:17 PDT
Created attachment 645536 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)
Comment 223 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:37:29 PDT
Created attachment 645537 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (r=smaug)
Comment 224 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:38:09 PDT
Created attachment 645539 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs (r=smaug)
Comment 225 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:39:30 PDT
Created attachment 645540 [details] [diff] [review]
part.8-1 Drop legacy mouse scroll events from IPC (r=smaug)
Comment 226 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:40:06 PDT
Created attachment 645542 [details] [diff] [review]
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug)
Comment 227 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:41:31 PDT
Created attachment 645544 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)
Comment 228 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:45:57 PDT
Created attachment 645546 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed
Comment 229 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:46:39 PDT
Created attachment 645547 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs
Comment 230 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:48:05 PDT
Created attachment 645549 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent
Comment 231 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:50:27 PDT
Created attachment 645551 [details] [diff] [review]
part.8-9 Handle WheelEvent.deltaZ in ESM (r=smaug)
Comment 232 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:52:07 PDT
Created attachment 645555 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler
Comment 233 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-07-24 15:53:22 PDT
Created attachment 645558 [details] [diff] [review]
part.13 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on GTK (r=karlt)
Comment 234 Steven Michaud [:smichaud] (Retired) 2012-08-06 10:03:27 PDT
Comment on attachment 639242 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug)

This looks fine to me (though I haven't tried to build or test it).  But I do have a couple of suggestions:

+#ifndef NP_NO_CARBON
+- (void)sendCarbonWheelEvent:(SInt)delta
+                     forAxis:(EventMouseWheelAxis)carbonAxis;
+#endif

+- (void)sendCarbonWheelEvent:(SInt)delta
+                     forAxis:(EventMouseWheelAxis)carbonAxis

Shouldn't "SInt" be "SInt32"?

+  // overflowDeltaX tells us when the user has tried to scroll past the edge
+  // of a page (in those cases it's non-zero).
+  // It only means left/right overflow when Gecko is processing a horizontal
+  // event.

I think this should be changed to something like:

"overflowDeltaX tells us when the user has tried to scroll past the
edge of a page to the left or the right (in those cases it's
non-zero)."
Comment 235 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-07 08:11:44 PDT
Comment on attachment 645546 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed


>+  nsEventStatus statusX = *aStatus;
>+  nsEventStatus statusY = *aStatus;
>+  if (scrollDeltaY) {
>+    SendLineScrollEvent(aTargetFrame, aEvent, &statusY,
>+                        scrollDeltaY, DELTA_DIRECTION_Y);
>+    if (!targetFrame.IsAlive()) {
>+      *aStatus = nsEventStatus_eConsumeNoDefault;
>+      return;
>+    }
>+  }
>+
>+  if (pixelDeltaY) {
>+    SendPixelScrollEvent(aTargetFrame, aEvent, &statusY,
>+                         pixelDeltaY, DELTA_DIRECTION_Y);
So same nsEventStatus variable is used for line and pixel scroll events.
Is that what happens currently too?
Comment 236 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-07 08:24:54 PDT
Hey, could you provide yet some new tryserver builds. I could test them for few days.
Comment 237 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-07 08:30:37 PDT
Comment on attachment 645555 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler

Make sure to re-trigger these tests few times on tryserver.
Comment 238 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-07 08:36:34 PDT
Comment on attachment 645547 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs


>+  if (mHandlingDeltaMode != PR_UINT32_MAX) {
Could you add a comment what PR_UINT32_MAX value means here.
Comment 239 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-07 19:13:13 PDT
(In reply to Olli Pettay [:smaug] from comment #235)
> Comment on attachment 645546 [details] [diff] [review]
> part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event
> into system group if the wheel event isn't consumed
> 
> 
> >+  nsEventStatus statusX = *aStatus;
> >+  nsEventStatus statusY = *aStatus;
> >+  if (scrollDeltaY) {
> >+    SendLineScrollEvent(aTargetFrame, aEvent, &statusY,
> >+                        scrollDeltaY, DELTA_DIRECTION_Y);
> >+    if (!targetFrame.IsAlive()) {
> >+      *aStatus = nsEventStatus_eConsumeNoDefault;
> >+      return;
> >+    }
> >+  }
> >+
> >+  if (pixelDeltaY) {
> >+    SendPixelScrollEvent(aTargetFrame, aEvent, &statusY,
> >+                         pixelDeltaY, DELTA_DIRECTION_Y);
> So same nsEventStatus variable is used for line and pixel scroll events.
> Is that what happens currently too?

Yes. And I think that it's good behavior.
http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#3316
Comment 240 Jim Mathies [:jimm] 2012-08-08 04:41:55 PDT
(In reply to Olli Pettay [:smaug] from comment #236)
> Hey, could you provide yet some new tryserver builds. I could test them for
> few days.

Same here!
Comment 241 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:49:13 PDT
(In reply to Jim Mathies [:jimm] from comment #240)
> (In reply to Olli Pettay [:smaug] from comment #236)
> > Hey, could you provide yet some new tryserver builds. I could test them for
> > few days.
> 
> Same here!

Here:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-d7da8d83a2ef/
Comment 242 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:55:21 PDT
And I'm testing if there is random orange in the new tests.
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d7da8d83a2ef
Comment 243 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:57:08 PDT
Created attachment 650054 [details] [diff] [review]
part.1 Add DOM3 WheelEvent (r=smaug, sr=jst)

Updated for latest trunk (including to replace nsnull with nullptr).
Comment 244 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:57:51 PDT
Created attachment 650055 [details] [diff] [review]
part.2 Separate code for deciding scroll target from nsEventStateManager::DoScrollText() (r=smaug)
Comment 245 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:58:27 PDT
Created attachment 650057 [details] [diff] [review]
part.3 Use ComputeScrollTarget() for deciding detail value of legacy mouse scroll events (r=smaug)
Comment 246 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:59:01 PDT
Created attachment 650058 [details] [diff] [review]
part.4 Remove mousewheel.*.*numlines and add mousewheel.*.delta_multiplier_* (r=smaug)
Comment 247 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 05:59:34 PDT
Created attachment 650059 [details] [diff] [review]
part.5 Redesign mouse wheel action prefs (r=smaug)
Comment 248 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:00:06 PDT
Created attachment 650061 [details] [diff] [review]
part.6 Separate pixel delta value accumulation code (r=smaug)
Comment 249 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:00:39 PDT
Created attachment 650062 [details] [diff] [review]
part.7 Drop NS_QUERY_SCROLL_TARGET_INFO handler from ESM (r=smaug)
Comment 250 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:01:18 PDT
Created attachment 650064 [details] [diff] [review]
part.8-2 GetScrollAmount() should return both width and height for oblique wheel event (r=smaug)
Comment 251 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:01:48 PDT
Created attachment 650065 [details] [diff] [review]
part.8-3 Redesign nsMouseWheelTransaction for D3E WheelEvent (r=smaug)
Comment 252 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:02:23 PDT
Created attachment 650066 [details] [diff] [review]
part.8-4 Implement D3E WheelEvent handler for scroll (r=smaug)
Comment 253 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:03:07 PDT
Created attachment 650067 [details] [diff] [review]
part.8-5 Dispatch legacy mouse scroll events before dispatching wheel event into system group if the wheel event isn't consumed (r=smaug)
Comment 254 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:03:48 PDT
Created attachment 650070 [details] [diff] [review]
part.8-6 Init lineDeltaX and lineDeltaY from accumulated delta values if the wheel event is caused by pixel scroll only device or the delta values have been modified with prefs (r=smaug)
Comment 255 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:04:26 PDT
Created attachment 650071 [details] [diff] [review]
part.8-7 Implement the other default actions of WheelEvent (r=smaug)
Comment 256 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:04:59 PDT
Created attachment 650072 [details] [diff] [review]
part.8-8 widget should be able to speicify the scroll type (r=smaug)
Comment 257 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:05:40 PDT
Created attachment 650073 [details] [diff] [review]
part.8-10 Remove the code handling legacy mouse events in layout (r=smaug)
Comment 258 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:06:14 PDT
Created attachment 650074 [details] [diff] [review]
part.9 Implement nsIDOMWindowUtils::sendWheelEvent() for tests (r=smaug)
Comment 259 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:07:02 PDT
Created attachment 650075 [details] [diff] [review]
part.10 Add new tests and remove tests for old ESM's legacy mouse scroll event handler (r=smaug)
Comment 260 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:07:58 PDT
Created attachment 650076 [details] [diff] [review]
part.12 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Windows (r=smaug+jimm)
Comment 261 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:08:42 PDT
Created attachment 650077 [details] [diff] [review]
part.14 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on Cocoa (r=smaug+smichaud)
Comment 262 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:09:56 PDT
Created attachment 650080 [details] [diff] [review]
part.17 Replace legacy mouse scroll event dispatchers with D3E wheel event dispatcher on nsWidgetUtils (r=smaug)
Comment 263 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 06:10:43 PDT
Created attachment 650081 [details] [diff] [review]
part.18 Clean up legacy mouse scroll event (r=smaug)
Comment 264 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 18:00:36 PDT
The results of the tests fine:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d7da8d83a2ef
Comment 265 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 18:57:26 PDT
I don't land them today. If you want to test with the tryserver builds, please do it ASAP. If you're busy and you want to test it before landing, let me know. I don't mind to wait a couple of days.
Comment 266 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-08 23:00:02 PDT
Created attachment 650444 [details]
testcase #2

This test logs wheel, DOMMouseScroll and MozMousePixelScroll.
Comment 267 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-09 10:53:36 PDT
Could you wait for few days before landing, like until the end of this week.
(Sunday might be anyway a good day to land big stuff like this.)
Comment 268 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-09 18:01:04 PDT
Indeed.
Comment 269 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-11 18:49:58 PDT
https://hg.mozilla.org/mozilla-central/rev/61d8207c4fa3
https://hg.mozilla.org/mozilla-central/rev/d1f62bfbc5e4
https://hg.mozilla.org/mozilla-central/rev/514cd0d4702e
https://hg.mozilla.org/mozilla-central/rev/863b63c60cdb
https://hg.mozilla.org/mozilla-central/rev/6a4cc533c88b
https://hg.mozilla.org/mozilla-central/rev/9953c53532ab
https://hg.mozilla.org/mozilla-central/rev/2d5c36f4d6db
https://hg.mozilla.org/mozilla-central/rev/28a9293fb872
https://hg.mozilla.org/mozilla-central/rev/f8dc49fed352
https://hg.mozilla.org/mozilla-central/rev/745475f156d2
https://hg.mozilla.org/mozilla-central/rev/81a0791331da
https://hg.mozilla.org/mozilla-central/rev/8a59a17c195c
https://hg.mozilla.org/mozilla-central/rev/842d2eaa63bc
https://hg.mozilla.org/mozilla-central/rev/b9472dc9837c
https://hg.mozilla.org/mozilla-central/rev/c648f9903e58
https://hg.mozilla.org/mozilla-central/rev/db912be9bc19
https://hg.mozilla.org/mozilla-central/rev/baeed7c05786
https://hg.mozilla.org/mozilla-central/rev/9f71b2d92ca5
https://hg.mozilla.org/mozilla-central/rev/70f1befd3216
https://hg.mozilla.org/mozilla-central/rev/e978181b5940
https://hg.mozilla.org/mozilla-central/rev/738efdde4d4c
https://hg.mozilla.org/mozilla-central/rev/4dd145ea74d2
https://hg.mozilla.org/mozilla-central/rev/e9a626db6c69
https://hg.mozilla.org/mozilla-central/rev/a43a1923ccea
https://hg.mozilla.org/mozilla-central/rev/208dc5fa121b
https://hg.mozilla.org/mozilla-central/rev/99f8bbe053e2
https://hg.mozilla.org/mozilla-central/rev/07819a70a6a5
https://hg.mozilla.org/mozilla-central/rev/83eeedda95c2
Comment 270 Ryan VanderMeulen [:RyanVM] 2012-08-11 21:03:47 PDT
A follow-up bustage fix was also pushed because this was landed directly on m-c after bug 773842 had been landed on inbound which had enabled FAIL_ON_WARNINGS in a few directories in content/. Needless to say, the next merge lit the tree up in flames. Let this serve as yet another warning for why using inbound is recommended over landing directly on m-c.

https://hg.mozilla.org/mozilla-central/rev/df8bdd5813f0
Comment 272 Sonny Piers [:sonny] 2012-08-14 01:29:02 PDT
Cool!

A little feedback:

On OSX where the scroll is inverted ('kinetic scrolling'), the webkit wheelevent event gives a inverted deltaY value where the Firefox wheel event doesn't.

I think the Firefox behavior is the correct one but it might need to be discussed.

What do you think?
Comment 273 Sonny Piers [:sonny] 2012-08-14 01:41:54 PDT
Actually maybe not because there is no way to know whether the user configuration is classic or kinetic scrolling. So giving the delta according to the scrolling direction might be the correct behavior.

My use case is handling the scrolling of an area myself. (so I can hide the scrollbars)
Comment 274 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-08-14 02:53:46 PDT
Sonny, thanks for testing. Could you please file a new bug about that OSX issue.
Make that bug block this one. CC me and masauyki.
Comment 276 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-31 02:19:01 PDT
FYI: The type of deltaX, deltaY and deltaZ are now defined as double in the latest draft.
Comment 277 chAlx 2012-12-03 03:09:56 PST
And how do i scroll a page up/down using mouse now?
Comment 278 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-12-03 18:18:56 PST
(In reply to chAlx from comment #277)
> And how do i scroll a page up/down using mouse now?

If you set delta_multiplier less than 100000 and a native wheel event's scroll amount is over one page, then, the actual scroll amount is limilted to one page. See bug 801101 for more detail.
Comment 279 dickvl 2012-12-21 12:58:32 PST
Noticed a few typos in this article:
https://developer.mozilla.org/en-US/docs/Mozilla_event_reference/wheel

implemenation instead of implementation (deltaX, deltaY, deltaZ)
deltaX	float	Expected amount that the page will scroll along the x-axis according to the deltaMode units; or an implemenation-specific value of movement of a wheel around the x-axis. Read Only.

consective should probably be consecutive:
# A vertical DOMMouseScroll event in both event group if accumulated deltaY of consective wheel events become larger than 1.0 or less than -1.0
# A vertical MozMousePixelScroll event in both event group if deltaY of the latest wheel event isn't zero
# A horizontal DOMMouseScroll event in both event group if accumulated deltaX of consective wheel events become larger than 1.0 or less than -1.0
# A horizontal MozMousePixelScroll event in both event group if deltaX of the latest wheel event isn't zero
Comment 280 Jean-Yves Perrier [:teoli] 2012-12-21 13:34:15 PST
Fixed, thanks.
Comment 281 danya.postfactum 2013-09-12 04:38:14 PDT
Masayuki, could you please comment your choice to prevent forced convertion lines->pixels (so, deltaMode would always be DOM_DELTA_PIXEL) here - http://code.google.com/p/chromium/issues/detail?id=227454 ?
(Personally I need DOM_DELTA_LINE when available, but there are some cases we need pixels (eg custom scrollbars))
It would be great if you share your thoughts about discussed issue.
Comment 282 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-09-12 08:17:34 PDT
(In reply to danya.postfactum from comment #281)
> Masayuki, could you please comment your choice to prevent forced convertion
> lines->pixels (so, deltaMode would always be DOM_DELTA_PIXEL) here -
> http://code.google.com/p/chromium/issues/detail?id=227454 ?
> (Personally I need DOM_DELTA_LINE when available, but there are some cases
> we need pixels (eg custom scrollbars))
> It would be great if you share your thoughts about discussed issue.

Browsers should provide loss-less information to web applications.

If web applications need to convert line or page to pixels, then, they should define its line-height, page-width or page-height.

Note You need to log in before you can comment on or make changes to this bug.