Closed
Bug 1253975
Opened 9 years ago
Closed 8 years ago
[e10s] <select>'s drop-down list scrolls as close as possible to the end, not beginning
Categories
(Core :: Layout: Form Controls, defect, P1)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla50
People
(Reporter: arni2033, Assigned: enndeakin)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: e10st?)
Attachments
(5 files, 1 obsolete file)
>>> My Info: Win7_64, Nightly 47, 32bit, ID 20160229030448
STR:
1. Open "testcase 1"
2. Click the <select>'s dropmarker
AR:
The drop-down list is scrolled as close to the end as possible. Selected option is visible
ER:
The drop-down list should be scrolled as close to the beginning as possible, because it matches
(1) non-e10s, (2) menulists in about:preferences#content -> Advanced, (3) GoogleChrome
(4) because there may be some <optgroup>s like in attachment 8672344 [details]
On that page, user can't see the name of oprgroup when <select> is just opened
I investigated them using the Browser Toolbox (remote debugging).
In case of non-e10s, elements are structured by <select><option>. But in case of e10s, elements are structured by <menulist><menupopup><menuitem>. So, e10s is affected by active theme. Is this correct?
I see that difference too, but I don't understand what did you mean by "e10s is affected by active theme". Because, yes, CSS related to XUL menulists applies to those "options" too. But this bug is about initial scroll position, I just don't see how styling issues are related
Just in case: bug 910022 will allow styling of those menuitems in multi-process mode.
Also, (2) is incorrect. Those menulists just scroll to the very beginning when I open it. Huh.
Comment 6•9 years ago
|
||
In my opinion, all are handling incorrectly. Ideally, the scroll position of a pre-selected item should be as close to center of the list view, as possible, not shifted to top or bottom of the list view. In my opinion this would better show context of the selected item within the list.
However, to be in parody with non-e10s and Chrome, e10s should shift the selected item to the top of the list view.
Updated•9 years ago
|
Blocks: e10s-select
Comment 7•9 years ago
|
||
Comment 9•9 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #8)
> jeff, does this block?
No, the web content is not broken, it is not even effectively different ( selected option is preserved correctly ) it is instead a visual discrepancy.
I would reconsider if there was an example where an interaction model ( mouse, kb, a11y, whatever ) was degraded in usefulness significantly.
Flags: needinfo?(jgriffiths)
Priority: -- → P1
Updated•9 years ago
|
tracking-e10s:
--- → +
Updated•8 years ago
|
Whiteboard: e10st?
Assignee | ||
Comment 10•8 years ago
|
||
For menulists, don't reset the scroll position to 0 when they open. This ensures that the later call to nsMenuPopupFrame::EnsureMenuItemIsVisible isn't overridden.
This means that the call to scrollIntoView isn't needed anymore.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•8 years ago
|
||
The tests in this patch depend on 1128156. Both patches together improve scrolling.
Depends on: 1128156
Assignee | ||
Comment 12•8 years ago
|
||
Attachment #8770652 -
Attachment is obsolete: true
Attachment #8771201 -
Flags: review?(mconley)
Comment 13•8 years ago
|
||
Comment on attachment 8771201 [details] [diff] [review]
Update scroll position properly, with null check
Review of attachment 8771201 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/base/content/test/general/browser_selectpopup.js
@@ +395,5 @@
> + let bpBottom = parseFloat(cs.paddingBottom) + parseFloat(cs.borderBottomWidth);
> +
> + is(selectPopup.childNodes[60].getBoundingClientRect().bottom,
> + selectPopup.getBoundingClientRect().bottom - bpBottom,
> + "Popup scroll at correct position " + bpBottom);
We're using top to calculate positioning in test_menulist_paging.xul - is there a reason why we're using bottom over here?
::: toolkit/content/tests/chrome/test_menulist_paging.xul
@@ +125,5 @@
>
> + let cs = window.getComputedStyle(menulist.menupopup);
> + let bpTop = parseFloat(cs.paddingTop) + parseFloat(cs.borderTopWidth);
> +
> + // Skip menulist3 as it has a label that scrolling doesn't need normally deal with.
Can you go into more detail about what "scroll" means?\ in the tests array schema? Like, it means the index of a child that we expect to be at the top of the menulist, right?
Attachment #8771201 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 14•8 years ago
|
||
> We're using top to calculate positioning in test_menulist_paging.xul - is
> there a reason why we're using bottom over here?
>
The menulist scrolls such that the bottom of the item is aligned with the bottom of the popup (as the bug title suggests). I could do the same for the other tests I guess, but why not test both top and bottom anyway.
Assignee | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/59ab86516175cf922f7de494d2a628cc8719b480
Bug 1253975, don't reset the scroll position of a menulist when it opens as it should scroll to its selection instead, r=mconley
Comment 16•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•