Closed
Bug 1197913
Opened 9 years ago
Closed 8 years ago
[e10s] Moving the cursor outside the <select> drop-down list doesn't keep the last hovered item highlighted
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
VERIFIED
FIXED
mozilla48
People
(Reporter: epinal99-bugzilla2, Assigned: ting)
References
(Blocks 1 open bug)
Details
(Whiteboard: [mozfr-community])
Attachments
(2 files, 3 obsolete files)
4.05 KB,
patch
|
enndeakin
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
8.27 KB,
image/png
|
Details |
I'm surprised this issue hasn't been reported yet, so maybe it's a dupe even if I checked bug 1154677. STR: 1) Open testcase https://bug1165782.bmoattachments.org/attachment.cgi?id=8606895 2) Click on the select (you can mouse up) 3) Move the cursor across the items then outside the drop-down list Result: The last item hovered doesn't stay highlighted after the mouse cursor goes outside the drop-down list.
Updated•9 years ago
|
tracking-e10s:
--- → +
Comment 1•8 years ago
|
||
Reproducible on all OS (Lin/OSX/Win) with aurora46. (Mozilla/5.0 (X11; Linux i686; rv:46.0) Gecko/20100101 Firefox/46.0 20160208004010 ) Result: E10s enabled - The last item hovered doesn't stay highlighted after the mouse cursor goes outside the drop-down list. Result: E10s dissabled - The last item hovered stays highlighted after the mouse cursor goes outside the drop-down list.
Updated•8 years ago
|
Assignee | ||
Comment 2•8 years ago
|
||
It's unhighlighted by https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsMenuFrame.cpp#471. I don't know how will it impact if the behavior is changed. For non-e10s, nsListControlFrame::MouseMove does not change selection when the mouse event is outside of the rectangle: https://dxr.mozilla.org/mozilla-central/source/layout/forms/nsListControlFrame.cpp#1865-1866 https://dxr.mozilla.org/mozilla-central/source/layout/forms/nsListControlFrame.cpp#1741-1743
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Ting-Yu Chou [:ting] from comment #2) > It's unhighlighted by > https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsMenuFrame. > cpp#471. I don't know how will it impact if the behavior is changed. Neil, what do you think if this line is removed? Will it break anything you know? Thanks.
Flags: needinfo?(enndeakin)
Comment 4•8 years ago
|
||
It might be ok, but will need some detailed testing for instance when submenus are present and/or disabled items nearby. Note though that in the brave new world, where we try to emulate native widget behaviour instead of the arbitrary behaviour of the non-e10s select element, this bug only applies to menulists and only on Windows, so the possibilities of noticeable issues are more limited.
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 5•8 years ago
|
||
I just did some tests see how does the thing work on different places: Keep it highlighted: Windows 10 native widget Chrome (Windows, and Ubuntu) Unhighlighted: Utubnut native widget Safari IE Edge
See Also: → 1053981
Updated•8 years ago
|
Assignee: nobody → janus926
Assignee | ||
Comment 6•8 years ago
|
||
I am not sure how to better test it, I ran this test: toolkit/content/tests/chrom/test_menu.xul which has submenu and disabled items I added. BTW, one thing I noticed on Windows is when cursor moves outside of a menu, the last hover item will be unhighlighted, which is different from selector.
Attachment #8732069 -
Flags: review?(enndeakin)
Assignee | ||
Comment 7•8 years ago
|
||
Accidentally included test code, removed it.
Attachment #8732069 -
Attachment is obsolete: true
Attachment #8732069 -
Flags: review?(enndeakin)
Attachment #8732072 -
Flags: review?(enndeakin)
Comment 8•8 years ago
|
||
Comment on attachment 8732072 [details] [diff] [review] patch v2 Review of attachment 8732072 [details] [diff] [review]: ----------------------------------------------------------------- No, as I said the new behaviour only applies to Windows and only to items in menulists. The current behaviour is correct for regular menus on Windows, and all cases on Mac and GTK. On Windows, you want to check if menuParent->GetParent() can QI to nsIDOMXULMenuListElement and not call ChangeMenuItem in this case.
Attachment #8732072 -
Flags: review?(enndeakin) → review-
Assignee | ||
Comment 9•8 years ago
|
||
So we will still have different behavior between e10s and non-e10s on Linux and Mac.
Attachment #8732072 -
Attachment is obsolete: true
Attachment #8732707 -
Flags: review?(enndeakin)
Comment 10•8 years ago
|
||
Comment on attachment 8732707 [details] [diff] [review] patch v3 Might be good to have a test.
Attachment #8732707 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Neil Deakin from comment #10) > Might be good to have a test. I will work out one.
Assignee | ||
Comment 12•8 years ago
|
||
Added a test which checks whether the last hover item is highlighted. I am not sure should this go to mochitest.ini, browser.ini, or chrome.ini. Thank you. :)
Attachment #8732707 -
Attachment is obsolete: true
Attachment #8734674 -
Flags: review?(enndeakin)
Updated•8 years ago
|
Attachment #8734674 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 13•8 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a360b638081
Assignee | ||
Comment 15•8 years ago
|
||
This is fixed on Windows only, see comment 4, 8 and 9 for the reason.
Comment 16•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d820bfb71d3a
Keywords: checkin-needed
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d820bfb71d3a
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 18•8 years ago
|
||
Ting-Yu, should your bug fix be uplifted to Aurora 47 in preparation for our e10s experiments on Beta 47?
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8734674 [details] [diff] [review] patch v4 Approval Request Comment [Feature/regressing bug #]: 897060 [User impact if declined]: different <select> drop-down list behavior on Windows between e10s and non-e10s/native widget [Describe test coverage new/current, TreeHerder]: automated, also added a test to make sure it behaves as expected [Risks and why]: low, it's just to keep the last hovered drop-down list item highligted after moving the cursor outside [String/UUID change made/needed]: n/a
Flags: needinfo?(janus926)
Attachment #8734674 -
Flags: approval-mozilla-aurora?
Comment on attachment 8734674 [details] [diff] [review] patch v4 e10s related, Aurora47+
Attachment #8734674 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I don't think we are taking any uplifts to Beta 46 for e10s related regressions. Updating the status field to reflect that.
Hello Loic, here is another bug that could use your verification? Please use the latest Nightly build as this hasn't been uplifted to Aurora channel yet. Many thanks for your ever awesome help!
Flags: needinfo?(epinal99-bugzilla2)
Comment 23•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/f4cacfbeef6b
Reporter | ||
Comment 24•8 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #22) > Hello Loic, here is another bug that could use your verification? Please use > the latest Nightly build as this hasn't been uplifted to Aurora channel yet. > Many thanks for your ever awesome help! The bug is partially fixed: when moving the cursor outside the dropdown list, the hovered option stays highlighted but not the dropdown arrow of the <select>. I'll file a new bug for the remaining issue.
Flags: needinfo?(epinal99-bugzilla2)
Verified on Nightly, thanks to Loic!
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•