Closed
Bug 1252723
Opened 8 years ago
Closed 8 years ago
[e10s] Make toolkit/components/prompts/test work under e10s
Categories
(Toolkit :: General, defect)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: adw, Assigned: adw)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
[e10s] Make toolkit/components/prompts/test work under e10s -- at least the following tests: test_bug619644.html test_bug620145.html test_bug625187.html test_bug861605.html Dolske's signed up for the other two tests in this directory.
Assignee | ||
Updated•8 years ago
|
Blocks: e10s-tests
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/37541/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37541/
Attachment #8725507 -
Flags: review?(dolske)
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8725507 [details] MozReview Request: Bug 1252723 - [e10s] Make toolkit/components/prompts/test work under e10s. r?dolske Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37541/diff/1-2/
Assignee | ||
Comment 3•8 years ago
|
||
The first patch fixed e10s but broke non-e10s, whoops. Second patch fixes that.
Assignee | ||
Comment 4•8 years ago
|
||
We should be able to remove prompt_common.js after all tests are converted, too.
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=de3981f94590
Assignee | ||
Comment 6•8 years ago
|
||
Oh, the summary of changes is pretty simple: These tests all use synthesizeKey() to close tab-modal prompts from mochitests running in content. The patch uses a chrome script to do that now. I couldn't figure out how to properly dispatch a key event to prompts, so I just call onButtonClick(1) on them.
Updated•8 years ago
|
tracking-e10s:
--- → +
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8725507 [details] MozReview Request: Bug 1252723 - [e10s] Make toolkit/components/prompts/test work under e10s. r?dolske Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37541/diff/2-3/
Assignee | ||
Comment 8•8 years ago
|
||
Forgot to remove the e10s skip-if from the mochitest.ini. https://treeherder.mozilla.org/#/jobs?repo=try&revision=061bb47d671e
Updated•8 years ago
|
Attachment #8725507 -
Flags: review?(dolske)
Comment 9•8 years ago
|
||
Comment on attachment 8725507 [details] MozReview Request: Bug 1252723 - [e10s] Make toolkit/components/prompts/test work under e10s. r?dolske https://reviewboard.mozilla.org/r/37541/#review37815 Sorry for the absurd delay. :( Would be r+, except for the first comment about the ESC keypress being needed for bug 619644's test. ::: toolkit/components/prompts/test/chromeScript.js:31 (Diff revision 3) > + infoTitle: { > + hidden: prompts[0].ui.infoTitle.getAttribute("hidden") == "true", > + }, > + }, > + }); > + prompts[0].onButtonClick(1); Did you perhaps try loading EventUtils into the chrome environment? I see a couple of tests (code_frame-script.js, browser_newtab_sponsored_icon_click.js) that do something similar... Would be nice if that worked. The bug 619644 test really needs to be an actual ESC keypress, since that bugfix is checking to see that the event.stopPropigation() call actually worked. (Might be easier to convert this test to be a browser-chrome test?) Otherwise this is fine for the other tests, which just want to close the prompt. I was a bit concerned about the way ConfirmEx prompts can have wonky button order/defaults... Was looking at if onKeyAction("cancel") would be better, although it needs an event, but I see it ultimately just calls onButtonClick(1) anyway. Which is what you're already doing. ::: toolkit/components/prompts/test/test_bug620145.html:95 (Diff revision 3) > var utils = SpecialPowers.getDOMWindowUtils(win); > utils.dispatchDOMEventViaPresShell(target, e, true); > ok(true, type + " sent to " + target.id); > } > > -function handleDialog(ui, testNum) > +function handleDialog() Since "handleDialog" is normally a function called by the prompt_common.js / startCallbackTimer() stuff -- which you've removed -- let's rename this to avoid any confusion... checkSelection() or your favorite bikeshed color. :) ::: toolkit/components/prompts/test/test_bug620145.html (Diff revision 3) > synthesizeMouse($("text"), 25, 55, { type: "mousemove" }); > is(window.getSelection().toString(), "", "selection not made"); > } > - > - ok(true, "handleDialog sending mouseclick to dialog..."); > - synthesizeMouse(ui.button0, 5, 5, { }, SpecialPowers.unwrap(ui.button0.ownerDocument.defaultView)); Ha, I guess it's a happy accident that your code "clicking" button1 still closes an alert(), which only has a button0. Could change the alert() to a confirm(), but it doesn't really matter. ::: toolkit/components/prompts/test/test_bug625187.html:84 (Diff revision 3) > > + chrome.destroy(); > SimpleTest.finish(); > } > > function handleDialog(ui) Ditto on the function name here.
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8725507 [details] MozReview Request: Bug 1252723 - [e10s] Make toolkit/components/prompts/test work under e10s. r?dolske Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37541/diff/3-4/
Attachment #8725507 -
Flags: review?(dolske)
Assignee | ||
Comment 11•8 years ago
|
||
Ah, that works, thanks. I actually did something similar in a previous test that I should have remembered here.
Assignee | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6bdf47b4113
Comment 13•8 years ago
|
||
Comment on attachment 8725507 [details] MozReview Request: Bug 1252723 - [e10s] Make toolkit/components/prompts/test work under e10s. r?dolske https://reviewboard.mozilla.org/r/37541/#review39683
Attachment #8725507 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7e4c0bee56f3
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7e4c0bee56f3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•