Closed Bug 1311279 Opened 3 years ago Closed 3 years ago

[e10s] Can not scroll the drop-down list with click-and-drag


(Core :: Layout: Form Controls, defect)

52 Branch
Windows 7
Not set



Tracking Status
firefox53 --- fixed


(Reporter: over68, Assigned: enndeakin)


(Blocks 1 open bug)


(Keywords: multiprocess)


(4 files, 1 obsolete file)

Steps to reproduce:

1. Go to
2. Click-and-hold on the <select> element, and drag down the drop-down list.

Actual results:

can not scroll the drop-down list with click-and-drag.
Blocks: e10s-select
Ever confirmed: true
Keywords: multiprocess
Summary: [e10s] can not scroll the drop-down list with click-and-drag → [e10s] Can not scroll the drop-down list with click-and-drag
Assignee: nobody → enndeakin
This is needed as we need to capture the mouse after the event has been sent to the child process and the popup has opened, and the normal setCapture only works during mousedown.
Attachment #8811233 - Flags: review?(bugs)
The non-e10s select uses a model where scrolling occurs when moving the mouse outside the top and bottom edge of the popup, however it responds only to mousemove events, so stops scrolling when you aren't moving the mouse. I don't think that seems quite right and makes the scrolling a bit jumpy, so I included a timer to scroll while outside the top and edge whether moving or not.

Native Windows dropdowns appear to use a model where the scroll speed is dependent on the mouse pointer distance from the popup, but I don't think it is worth adding extra code to emulate that detail.
Attachment #8811235 - Flags: review?(mconley)
Attached file Test case
Moving the test mentioned in comment 0 from Dropbox to Bugzilla.
Comment on attachment 8811235 [details] [diff] [review]
Part 2 - scroll with click+drag on select

Review of attachment 8811235 [details] [diff] [review]:

Thanks for the great test too! This looks okay to me (modulo notes below).

::: browser/base/content/test/general/browser_selectpopup.js
@@ +425,5 @@
> +  // Check if a drag-select works and scrolls the list.
> +  yield openSelectPopup(selectPopup, "mousedown", "select", win);
> +
> +  let scrollPos = selectPopup.scrollBox.scrollTop;
> +  let popupRect = selectPopup.getBoundingClientRect()

Nit: missing semicolon

::: toolkit/modules/SelectParentHelper.jsm
@@ +65,5 @@
>      menupopup.setConstraintRect(constraintRect);
>      menupopup.openPopupAtScreenRect("after_start", rect.left,, rect.width, rect.height, false, false);
> +
> +    // Set up for dragging
> +    menupopup.setCaptureAlways();

I'm not 100% privy to how nsIPresShell content capturing works, but is it necessary to undo this setCaptureAlways in popuphidden?

@@ +76,5 @@
>        menulist.menupopup.hidePopup();
>      }
>    },
> +  clearScrollTimer: function() {

The user can close the popup without mouseup'ing (example, pressing Esc during a click-hold). Should we also clear this timer on popuphidden?

@@ +102,5 @@
>          break;
> +      case "mousemove":
> +        let menupopup = currentMenulist.menupopup;
> +        let popuprect = menupopup.getOuterScreenRect();

Nit: "popuprect" -> "popupRect".
Attachment #8811235 - Flags: review?(mconley) → review+
Attachment #8811233 - Flags: review?(bugs) → review+
Bug 1311279, add a chrome-only setCapture method that can ignore the allowed state, r=smaug
Bug 1311279, scroll the select popup when click+drag is used, r=mconley
Bug 1311279, add a chrome-only setCapture method that can ignore the allowed state, r=smaug
Bug 1311279, scroll the select popup when click+drag is used, r=mconley
Flags: needinfo?(enndeakin)
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1320565
Depends on: 1321472
backed out part 2 for intermittent failure from bug 1272834:
Resolution: FIXED → ---
This is just an updated version of the same patch.
Attachment #8811235 - Attachment is obsolete: true
This patch checks if the mouse was released over the select button area (in the mouseup event handler) and indicates in the message sent to the child process that mouseup/click events should be sent to the select element.

Most of the changes in SelectContentHelper.jsm are refactoring the event dispatch to a separate function dispatchMouseEvent. The real change is in "Forms:MouseUp" handler.

An additional check is added to the mousemove handler to cancel the drag processing if the mouse button is not pressed. This can happen if the mouse wasn
t used to open the dropdown or in some cases the timing of the events/messages between the processes can cause the mouseup to occur while the popup is opening and confuse things. The check isn't ideal but handles this well enough.

The test doesn't actually verify this change; but maybe one day BrowserTestUtils.synthesizeMouse will route through the parent process event handling and it will test this.
Attachment #8824234 - Flags: review?(mconley)
Comment on attachment 8824234 [details] [diff] [review]
Part 3 - additional patch to handle click event

Review of attachment 8824234 [details] [diff] [review]:

Looks okay to me (though I haven't reviewed part 2, which this seems to rely on). Thanks!
Attachment #8824234 - Flags: review?(mconley) → review+
Pushed by
scroll the select popup when click+drag is used, r=mconley
when the mouse is released, check if it should be retargetted at the select element in the content process, so that a click event is received, r=mconley
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.