1128156 - [e10s] unnecessary scroll up 1 line when mouse hover <select> dropdown list

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Alice0775 White]

Attachment: select_bug1.html

The Bug 1069463 again

Steps To Reproduce:
1. Open e10s window
2. Open attached and click dropdown marker of <select> element
3. Move mouse pointer to the bottom

Actual Results:
 Scroll up 1 line

Expected Results:
 Not scroll the list


Triggered by: Bug 1108761
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=33445508e525&tochange=dece5291d037

Comment 1

[Alice0775 White]

Maybe you need the following STR insted of the above, due to Bug 910022, 
  

Steps To Reproduce:
1. Open e10s window
2. Open attached and click the left half of the elements to expand dropdown list
3. Move mouse pointer to the bottom

Comment 2

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160205004003

I can confirm this issue is still reproducible on the latest Aurora 46.0a2 on Windows 7 when e10s is enabled (followed the steps from Comment 1). 

The issue not reproducible when e10s is disabled.

Comment 3

[Neil Deakin]

Attachment: Calculate a better maximum height for the popup

This patch changes the maximum height to be exactly 20 items, avoiding partially visible items that could cause scrolling.

The padding on Mac is moved to the popup itself so the popup-internal-box height which clips the items isn't 8 pixels extra, which would cause off-scroll items to appear in that eight pixel area.

Comment 4

[arni2033]

(In reply to Neil Deakin from comment #3)
> This patch changes the maximum height to be exactly 20 items, avoiding
> partially visible items that could cause scrolling.
Is your patch a confirmation that bug 910022 is wontfix? If in bug 910022 some styling (that allows to
change option's height) won't be broken in e10s, your solution won't help this bug ("bug"?) completely

Comment 5

[Neil Deakin]

It won't work if each option's height can be different no, but a fix for that could be made by iterating over each row and computing a suitable height if we did decide to support that.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8770647 [details] [diff] [review]
Calculate a better maximum height for the popup

Review of attachment 8770647 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/SelectParentHelper.jsm
@@ +36,5 @@
> +      // Include the padding and border on the popup.
> +      let cs = win.getComputedStyle(menulist.menupopup);
> +      let bpHeight = parseFloat(cs.borderTopWidth) + parseFloat(cs.borderBottomWidth) +
> +                     parseFloat(cs.paddingTop) + parseFloat(cs.paddingBottom);
> +      menulist.menupopup.style.maxHeight = (itemHeight * 20 + bpHeight) + "px";

Might as well define a constant somewhere in this JSM instead of magic-numbering 20 here.

Comment 7

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5b7fe7de013f16df861ec28c452eeb3f3a6270c
Bug 1128156, set the maximum height of the select popup to exactly 20 rows, r=mconley

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e5b7fe7de013

Comment 9

[Alexandru Simonca [:asimonca], Release Desktop QA]

This is VERIFIED FIXED on Firefox Beta 50.0b4 (id: 20161003155957) on Windows 7 x64.
Marking it accordingly.