Write new test for bug 299673
Categories
(Core :: DOM: UI Events & Focus Handling, defect, P5)
Tracking
()
People
(Reporter: MatsPalmgren_bugz, Unassigned)
References
Details
Attachments
(2 files, 4 obsolete files)
2.74 KB,
patch
|
Details | Diff | Splinter Review | |
3.75 KB,
text/html
|
Details |
I have disabled the mochitests for bug 299673: content/events/test/test_bug299673-1.html content/events/test/test_bug299673-2.html since they are perma/random-orange: bug 553344, bug 524361 and bug 521355. I tried to fix it using SimpleTest.waitForFocus but it didn't work. Filing this bug to re-write these tests from scratch.
Comment 1•14 years ago
|
||
(In reply to comment #0) http://hg.mozilla.org/mozilla-central/rev/22c8fbd5551b
Comment 2•8 years ago
|
||
This file: http://mxr.mozilla.org/mozilla-central/source/dom/events/test/bug299673.js has to be rewritten to use pushPrefEnv, but since it's disabled. http://mxr.mozilla.org/mozilla-central/source/dom/events/test/mochitest.ini#24 http://mxr.mozilla.org/mozilla-central/source/dom/events/test/test_bug299673-1.html?force=1 http://mxr.mozilla.org/mozilla-central/source/dom/events/test/test_bug299673-2.html?force=1
Comment 4•8 years ago
|
||
This makes the test pass locally for me on MacOSx10.10 debug.
Updated•8 years ago
|
Comment 5•8 years ago
|
||
Comment on attachment 8724778 [details] [diff] [review] 553417.diff Tryservers shows that the test is failing on linux, e10s and Windows. It's passing on Mac. Mats, is it ok to re-enable these tests like this for Mac, or did you mean something else with "re-write these tests from scratch"?
Reporter | ||
Comment 6•8 years ago
|
||
I think I meant that someone needs to investigate what the results for this test is supposed to be according to relevant specs (if any) and compare what other UAs are doing. It certainly seems wrong (to me) that we get different results on different platforms. It may be that the test is poorly written, or it may be an actual underlying bug here that we need to fix. What the test is doing is this: in the top document we have a <select size=1> and (after it) an <input type=text>. First we focus the <select>, then we synthesize a VK_DOWN (to change the selected value) and then a VK_TAB. The VK_TAB should cause a 'change' event followed by a 'blur' on the <select> and then 'focus' on the <input>. However, there is an onchange handler on the <select> that opens a popup window, which will blur the top document and its currently focused element, and focus its document instead. The question is, in what order should all these focus/blur events occur? Here the test results from (non-E10S) Linux32 (that failed), FTR: (edited for readability) =============================== FAIL | dom/events/test/test_bug299673-1.html | wrong element is focused after popup was closed - got [object HTMLInputElement], expected [object HTMLBodyElement] FAIL | dom/events/test/test_bug299673-1.html | expected events - got " : Test with browser.link.open_newwindow = 2 focus top-doc SELECT(Select1): focus SELECT(Select1): change >>> OpenWindow <<< OpenWindow SELECT(Select1): blur INPUT(Text1): focus INPUT(Text1): blur blur top-doc focus popup-doc INPUT(popupText1): focus INPUT(popupText1): blur blur popup-doc focus top-doc INPUT(Text1): focus", expected " : Test with browser.link.open_newwindow = 2 focus top-doc SELECT(Select1): focus SELECT(Select1): change >>> OpenWindow blur top-doc focus popup-doc INPUT(popupText1): focus <<< OpenWindow SELECT(Select1): blur INPUT(popupText1): blur blur popup-doc focus top-doc" =============================== The "OpenWindow" message is from the function that does the window.open() call: http://mxr.mozilla.org/mozilla-central/source/dom/events/test/bug299673.js#3 If you look at the actual result, it seems that the focus() calls inside the popup window are now asynchronous and that we return before reaching "focus popup-doc" etc. and that we process the TAB in the top document (SELECT(Select1): blur, INPUT(Text1): focus) before the focus stuff in the popup window occurs. The expected results seems to be expecting a more synchronous behavior, i.e. that the "SELECT(Select1): blur" happens after OpenWindow returns (which runs inside onchange). I don't know which behavior is "correct" here. (I guess it should be possible to run this test manually in other UAs by focusing the <select>, type the DOWN arrow key then TAB. It might need some tweaks though.) Also note that other UAs might fire the 'change' event already on the DOWN key. It should be possible to compare the order of events between the top/popup documents anyway I hope.
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8724778 [details] [diff] [review] 553417.diff The pushPrefEnv fix in bug299673.js looks good to me. (r=mats on that part) I don't think we should enable these tests until we know what the problem is and what other UAs do. (r- on those parts for now)
Comment 8•8 years ago
|
||
Ok, this is without enabling the tests.
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8726756 [details] [diff] [review] 553417.diff But the test is already enabled in the manifest, although at the moment it just runs that todo(). These changes *will* run the test AFAICT. Am I missing something?
Comment 10•8 years ago
|
||
You're right, sorry about that. I forgot about it, that this was disabled on the test level instead of the manifest level.
Reporter | ||
Comment 11•8 years ago
|
||
Comment on attachment 8726786 [details] [diff] [review] 553417.diff While we're here -- please add a ", see bug 553417" at the end of the todo() and leave this bug open after landing since it contains useful info on what remains to be done. Thanks. Also, you probably want to modify the commit message so that it only says "rewrite tests to use pushPrefEnv".
Comment 12•8 years ago
|
||
With comment 11 addressed.
Updated•8 years ago
|
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cc5bb93e4e9
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9cc5bb93e4e9
Updated•8 years ago
|
Comment 15•8 years ago
|
||
On Google Chrome and Safari, pressing the down key opens the select popup, and then the tab key doesn't do anything. You have to press the Return, space or esc key to get the select popup to be dismissed.
Comment 16•8 years ago
|
||
Microsoft Edge (and I think Internet Explorer as well) opens the popup window as soon as press the down key, the result after that is this: [object HTMLSelectElement]:(Select1): focus [object HTMLSelectElement]:(Select1): change >>> OpenWindow [object HTMLSelectElement]:(Select1): blur [object HTMLInputElement]:(popupText1): focus [object HTMLSelectElement]:(Select1): change top-doc <<< ClosePopup [object HTMLInputElement]:(popupText1): change popup-doc [object HTMLSelectElement]:(Select1): focus [object HTMLInputElement]:(popupText1): blur Regarding the results in comment 15, they are for Firefox trunk on Mac. I don't know about if there is any spec that is telling how UA's should behave here. I don't think there is. I currently am not able to test on Linux. Let me know if you need more info and what I need to do to enable these tests.
Reporter | ||
Comment 17•8 years ago
|
||
The HTML spec describes some events for <select>, I haven't looked in detail what it says though. And in our case there is also window focus management involved which I'm not sure how much is specified, if at all (the HTML spec is probably where it would be if it exists). https://html.spec.whatwg.org/multipage/forms.html#the-select-element > On Google Chrome and Safari, pressing the down key opens the select popup [...] Interesting. That's on OSX I presume. Chrome on Linux does NOT do that -- it changes the value to the next one in the list without opening the dropdown menu: Current output: [object HTMLSelectElement]:(Select1): focus [object HTMLSelectElement]:(Select1): change >>> OpenWindow [object HTMLSelectElement]:(Select1): change top-doc [object HTMLSelectElement]:(Select1): blur [object HTMLInputElement]:(popupText1): focus [object HTMLInputElement]:(popupText1): blur <<< ClosePopup [object HTMLSelectElement]:(Select1): focus > Microsoft Edge (and I think Internet Explorer as well) opens the popup window as soon as press > the down key OK, that's what we should do too on all platforms, except maybe OSX if KEY_DOWN generally opens the menu instead (in native UI etc) on that platform. Is there another key-combo to change the value directly (without opening the menu) on OSX? I think that's bug 126379. Unfortunately I won't have time to work on that for the foreseeable future. I'm not sure there is much value in these tests before that happens, but feel free to make it work and enable it on some platforms if you have the time. At least it protects us from regressing the behavior that we currently have, even it that behavior is currently wrong. rs=mats on anything that's green on Try.
Comment 18•8 years ago
|
||
(In reply to Mats Palmgren (:mats) from comment #17) > OK, that's what we should do too on all platforms, except maybe OSX if > KEY_DOWN generally > opens the menu instead (in native UI etc) on that platform. Is there > another key-combo > to change the value directly (without opening the menu) on OSX? pressing the 'o' key opens the new window popup and changes the select value. The output I get on Chrome is: [object HTMLSelectElement]:(Select1): focus [object HTMLSelectElement]:(Select1): change >>> OpenWindow [object HTMLSelectElement]:(Select1): change top-doc [object HTMLSelectElement]:(Select1): blur [object HTMLInputElement]:(popupText1): focus <<< ClosePopup [object HTMLSelectElement]:(Select1): focus [object HTMLSelectElement]:(Select1): blur
Reporter | ||
Comment 19•8 years ago
|
||
OK, so it does have that behavior for "changing the value using a kbd command" then, only it's not KEY_DOWN specifically. That's good to know, thanks. I'm even more convinced we need to fix bug 126379 then.
Comment 20•5 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months. :hsinyi, maybe it's time to close this bug?
Comment 21•5 years ago
|
||
There's still a open dependency bug. It doesn't sound right to close this. :mats, please feel free to chime in.
Assignee | ||
Updated•5 years ago
|
Comment 22•5 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
Comment 23•5 years ago
|
||
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #22)
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
no.
Comment 24•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
Comment 26•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:jstutte, maybe it's time to close this bug?
Updated•4 years ago
|
Comment 27•3 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:jstutte, maybe it's time to close this bug?
Updated•3 years ago
|
Comment 28•3 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
Updated•3 years ago
|
Comment 29•2 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.
Comment 30•2 years ago
|
||
(In reply to Release mgmt bot [:marco/ :calixte] from comment #29)
The leave-open keyword is there and there is no activity for 6 months.
:hsinyi, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.
No, let's keep this open.
Updated•2 years ago
|
Comment 31•1 year ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Comment hidden (spam) |
Updated•20 days ago
|
Description
•