1194027 - [e10s] DoubleClicking a select doesn't collapse when clicking on the dropmarker

Status: RESOLVED FIXED

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

Description

[arni2033]

STR:   (Win7, Nightly 43.0a1 (2015-08-12))
1. Open     https://bug1186972.bmoattachments.org/attachment.cgi?id=8638018 (attachment on bug 1186972)
   OR page  data:text/html,<select><option>First<option>Second
2. DoubleClick on dropmarker

Result:       The drop-down list hasn't collapsed
Expectations: It should've collapsed

// I have mentioned this in comments bug 1186972 comment 8, bug 1186972 comment 10

Comment 1

[Tracy Walker [:tracy]]

A double click is just the same as clicking it twice.  It will open, then close or close, then open depending on starting state.  This is how it works on Release.  And Nightly e10s performs exactly the same.

Comment 2

[arni2033]

Attachment: [e10s] DoubleClicking a select doesn't collapse when clicking on the dropmarker.webm

Please watch the video and tell what OS have you used for the testing

Comment 3

[Tracy Walker [:tracy]]

Ah, my apologies. So the start state is closed, then in the end it doesn't close for you.  I am on Mac OSX.  In the video, are you doing a third click away from the select item?  (making it close)

Comment 4

[arni2033]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #3)
> In the video, are you doing a third click away from the select item?  (making it close)
Yes, on the video I wait a bit, then click on the free area to show the issue again

Comment 5

[Karla Merza [Away. Please needinfo? Brindusa Tot[:brindusat] ]]

User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID 	20160207004019

Tested on Windows 7 x64 with the Aurora 46.0a2 build. With e10s enabled, I can sometimes reproduce the problem. With e10s disabled, the issue does not reproduce.

Comment 6

[Neil Deakin]

The issue here is in the NS_THEME_MENULIST_BUTTON block of nsNativeThemeWin::GetThemePartAndState

IsDroppedDown() is called but will never return true in e10s since we don't set the flag used by it.

The code here inside the IsVistaOrLater() block is a little odd in that it doesn't handle the non-open case and just falls through. In non-e10s you can see odd behaviour if you hold the mouse on the drop marker, then press escape, then move the mouse slightly -- the active state only appears then and no other time. So I don't think this code is correct as is anyway, but we probably will still need an IsDroppedDown method that returns the correct state.

Comment 7

[Neil Deakin]

Oops, that comment was meant for bug 1253979.

The comment for this bug is:

The non-e10s is case toggles the list open state within nsListControlFrame::MouseDown, however e10s only opens it. So might be able to fix this by using a message which toggles the state rather than only opens it.

Comment 8

[Cervantes Yu [:cyu] [:cervantes]]

This is a Windows-only bug. On Mac and Linux, the 2nd click in a double click is handled like a single click and short-circuit to nsXULPopupManager::Rollup() from nsWindow::CheckForRollup() (Linux) and [ChildView maybeRollup] (Mac). I guess we can do likewise in nsWIndow::DealWithPopups().

Comment 9

[Cervantes Yu [:cyu] [:cervantes]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a312fcb3ab2a

Comment 10

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: bug_1194027.patch

Comment 11

[Neil Deakin]

Comment on attachment 8772361 [details] [diff] [review]
bug_1194027.patch

This will cause all popups to rollup on double-click which we don't want to do in general.

Comment 12

[Neil Deakin]

Attachment: Add a chrome-only flag to select used to track if the parent process popup is open

You'll need 1159301 if you want to apply the test part of the patch.

Comment 13

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/69b3a93aeba57617eaa4c579bea5c78f343c7d43
Bug 1194027, add a flag to select elements to indicate if the parent process has the popup open, r=mrbkap

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/69b3a93aeba5

Comment 15

[Neil Deakin]

I didn't include the test for this patch as it was failing intermittently on Windows.