Port editor mochitest-chrome tests to mochitest-plain so they work on e10s

RESOLVED FIXED in Firefox 52

Status

()

Core
Editor
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

(Blocks: 1 bug)

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(26 attachments, 2 obsolete attachments)

917 bytes, patch
masayuki
: review-
Details | Diff | Splinter Review
9.59 KB, patch
masayuki
: review+
Details | Diff | Splinter Review
69.02 KB, patch
Details | Diff | Splinter Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
58 bytes, text/x-review-board-request
masayuki
: review+
Details | Review
So far I tried porting all the .html tests in editor/libeditor/tests/:
* test_bug46555.html: Pass
* test_bug366682.html: onSpellCheck is not defined.  AsyncSpellCheckHelper.jsm seems not to be imported properly, but I don't see any errors in the console.  Occurs with or without e10s.
* test_bug471319.html: Pass
* test_bug483651.html: Pass
* test_bug569988.html: With e10s, onPromptFocus() isn't called, test hangs.  Without e10s, 'Error: Permission denied to access property "Dialog"'
* test_bug635636.html: mainWindow.gBrowser is undefined, only with e10s
* test_bug830600.html: "Error: Unable to restore focus, expect failures and timeouts" followed by "WARNING: TabChild::SetFocus not supported in TabChild" and hang, only with e10s
* test_bug1053048.html: Pass
* test_bug1100966.html: Pass
* test_bug1101392.html: top.document.commandDispatcher is undefined, with or without e10s (SpecialPowers.wrap(top.document).commandDispatcher is also undefined)
* test_bug1102906.html: Depends on ChromeUtils.js
* test_bug1140105.html: Pass
* test_bug1153237.html: top.document.commandDispatcher is undefined, with or without e10s (SpecialPowers.wrap(top.document).commandDispatcher is also undefined)
* test_bug1154791.html: Pass
* test_bug1248128.html: top.document.commandDispatcher is undefined, with or without e10s (SpecialPowers.wrap(top.document).commandDispatcher is also undefined)
* test_bug1248185.html: top.document.commandDispatcher is undefined, with or without e10s (SpecialPowers.wrap(top.document).commandDispatcher is also undefined)
* test_bug1250010.html: Pass
* test_bug1257363.html: Pass
* test_composition_event_created_in_chrome.html: Pass
* test_contenteditable_text_input_handling.html: Pass
* test_dragdrop.html: Pass

These errors are probably all fixable.  Once I'm done with that, there's also editor/composer/test/, and then the .xul tests in both of them.
.html tests in editor/composer/test/:

* test_async_UpdateCurrentDictionary.html: Pass
* test_bug338427.html: Pass
* test_bug678842.html: With e10s, NS_ERROR_NOT_AVAILABLE from nsIEditorSpellCheck.SetCurrentDictionary.  Immediately prior, prints "WARNING: ENSURE_MAIN_PROCESS failed.  Cannot SetString from content process: spellchecker.dictionary"
* test_bug697981.html: With e10s, 'expected de-DE - got "en-US"' (.GetCurrentDictionary() returns the wrong result)
* test_bug717433.html: With e10s, NS_ERROR_NOT_AVAILABLE from nsIEditorSpellCheck.SetCurrentDictionary.  Immediately prior, prints "WARNING: ENSURE_MAIN_PROCESS failed.  Cannot SetString from content process: spellchecker.dictionary"
* test_bug1200533.html: With e10s, 'expected xx-XX, got "en-US"' x6 (.GetCurrentDictionary returns the wrong result)
* test_bug1204147.html: With e10s, "unexpected lang en-US instead of en-GB" (.GetCurrentDictionary() returns the wrong result) 
* test_bug1205983.html: With e10s, 'one misspelled word expected: German - got "heuteisteinguter", expected "German"' x2
* test_bug1209414.html: With e10s, NS_NO_INTERFACE at SpecialPowers.wrap(window) (line was present before I touched the test, but didn't cause the error)
* test_bug1219928.html: With e10s, NS_ERROR_ILLEGAL_VALUE returned by nsIEditorSpellCheck.GetNextMisspelledWord

It looks like some spellcheck-related stuff is very broken with e10s.

This concludes the first round of .html conversions.  I'll look at .xul now.

(In reply to :Aryeh Gregor (working until May 8) from comment #0)
> * test_bug366682.html: onSpellCheck is not defined. 
> AsyncSpellCheckHelper.jsm seems not to be imported properly, but I don't see
> any errors in the console.  Occurs with or without e10s.

I figured out how to fix this, but not the others yet.
I don't think the XUL tests are necessarily convertible to plain HTML files, e.g.:

ok(!threw, "The execCommand API should work on <xul:editor>");
http://hg.mozilla.org/mozilla-central/file/77cead2cd203/editor/composer/test/test_bug434998.xul

So I need help on how I'm supposed to proceed with the XUL files -- convert them to browser tests?  Leave them for now?  Does this really need to be tested in e10s to begin with?  (Ehsan apparently said all the XUL tests here should run in e10s.)
Flags: needinfo?(mrbkap)
Flags: needinfo?(felipc)
tracking-e10s: --- → ?
(In reply to :Aryeh Gregor (working until May 8) from comment #1)
> * test_bug678842.html: With e10s, NS_ERROR_NOT_AVAILABLE from
> nsIEditorSpellCheck.SetCurrentDictionary.  Immediately prior, prints
> "WARNING: ENSURE_MAIN_PROCESS failed.  Cannot SetString from content
> process: spellchecker.dictionary"
> * test_bug697981.html: With e10s, 'expected de-DE - got "en-US"'
> (.GetCurrentDictionary() returns the wrong result)
> * test_bug717433.html: With e10s, NS_ERROR_NOT_AVAILABLE from
> nsIEditorSpellCheck.SetCurrentDictionary.  Immediately prior, prints
> "WARNING: ENSURE_MAIN_PROCESS failed.  Cannot SetString from content
> process: spellchecker.dictionary"
> * test_bug1200533.html: With e10s, 'expected xx-XX, got "en-US"' x6
> (.GetCurrentDictionary returns the wrong result)
> * test_bug1204147.html: With e10s, "unexpected lang en-US instead of en-GB"
> (.GetCurrentDictionary() returns the wrong result) 
> * test_bug1205983.html: With e10s, 'one misspelled word expected: German -
> got "heuteisteinguter", expected "German"' x2

A lot of these tests are trying to install new spellcheck dictionaries. That can't happen from the child. You can probably create a helper that uses a chrome script [1] to install the dictionary and then continue running the test from the child.

> * test_bug1209414.html: With e10s, NS_NO_INTERFACE at
> SpecialPowers.wrap(window) (line was present before I touched the test, but
> didn't cause the error)

This is trying to find the chrome window in (now) a content process in order to install a context menu listener so this part of the code should move to the parent process via a chrome script along with the code that installs the new dictionary (probably via a spellcheck_common.js library).

> * test_bug1219928.html: With e10s, NS_ERROR_ILLEGAL_VALUE returned by
> nsIEditorSpellCheck.GetNextMisspelledWord

It isn't obvious to me what's wrong here. More debugging is necessary.

(In reply to :Aryeh Gregor (working until May 8) from comment #2)
For the specific example you cite here (<xul:editor>), I don't know how much sense it makes sense to test <xul:editor> functionality in e10s. We don't (AFAIK) use it in the browser and I don't see any way to have the inner, edited, document be loaded in a remote process (a la <xul:browser remote=true>).

Does that help?

[1] https://wiki.mozilla.org/Electrolysis/e10s_test_tips#Mochitest_.28plain.29
Flags: needinfo?(mrbkap) → needinfo?(ayg)
Thanks a lot!  Can you also help me out with my problems in comment #0?  (I already found a way around the first one -- assign the result of .import to a variable instead of relying on names being put in the global scope -- although I don't understand why it works.)
Flags: needinfo?(ayg) → needinfo?(mrbkap)
(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #3)
> A lot of these tests are trying to install new spellcheck dictionaries. That
> can't happen from the child. You can probably create a helper that uses a
> chrome script [1] to install the dictionary and then continue running the
> test from the child.

Thanks, that fixed all of them!

> This is trying to find the chrome window in (now) a content process in order
> to install a context menu listener so this part of the code should move to
> the parent process via a chrome script along with the code that installs the
> new dictionary (probably via a spellcheck_common.js library).

I get an error that window isn't defined.  Just in case, I tried passing the window object in the message, but naturally, it didn't work.
(In reply to :Aryeh Gregor (working until May 8) from comment #1)
> * test_bug1219928.html: With e10s, NS_ERROR_ILLEGAL_VALUE returned by
> nsIEditorSpellCheck.GetNextMisspelledWord

In a debugger, I couldn't even see that nsEditorSpellCheck::GetNextMisspelledWord was called, and I couldn't figure out how to get it to hit a breakpoint in the JS debugger.  I think I'll leave this one to a follow-up bug.
(In reply to :Aryeh Gregor (working until May 8) from comment #5)
> I get an error that window isn't defined.  Just in case, I tried passing the
> window object in the message, but naturally, it didn't work.

Yeah, you can't pass the window to the parent script. You'll have to get at the chrome window another way. I think you might be able to do something like |browserElement.ownerDocument.defaultView| in the chrome script. If that doesn't work, you can use Components.classes to get the window watcher service, from which you can get the most recent navigator window and go from there.
Flags: needinfo?(mrbkap)
Flags: needinfo?(felipc)
(In reply to :Aryeh Gregor (working until May 8) from comment #0)
> * test_bug366682.html: onSpellCheck is not defined. 
> AsyncSpellCheckHelper.jsm seems not to be imported properly, but I don't see
> any errors in the console.  Occurs with or without e10s.

At a guess, this has to do with the weirdness with variables object and import and all that fun stuff. Importing into an object is the right way to go.

> * test_bug569988.html: With e10s, onPromptFocus() isn't called, test hangs. 
> Without e10s, 'Error: Permission denied to access property "Dialog"'

I'm surprised that we call onPromptLoad. Does moving the onPromptLoad/onPromptFocus stuff into a chrome script help?

> * test_bug635636.html: mainWindow.gBrowser is undefined, only with e10s

This is getting the chrome window from mainWindow so it can set a pageshow listener for the new page (?). It should be possible to go from the window, via SpecialPowers, to the chrome event listener for the window. I don't know how to do that off the top of my head though and it isn't (yet) built into SP.

> * test_bug830600.html: "Error: Unable to restore focus, expect failures and
> timeouts" followed by "WARNING: TabChild::SetFocus not supported in
> TabChild" and hang, only with e10s

I have no idea without debugging this, sorry.

> * test_bug1101392.html: top.document.commandDispatcher is undefined, with or
> without e10s (SpecialPowers.wrap(top.document).commandDispatcher is also
> undefined)

commandDispatcher is only defined on XUL documents which cannot exist in the child process. Unless this test can be converted to use web APIs, I don't think it needs to be converted (testing a non-content-process API).

> * test_bug1102906.html: Depends on ChromeUtils.js

You might be able to import the synthesizeDrop stuff into specialpowersAPI.js pretty much wholesale (though it'll be some work and it'd be nice to modernize some of the code if you do).
(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #7)
> Yeah, you can't pass the window to the parent script. You'll have to get at
> the chrome window another way. I think you might be able to do something
> like |browserElement.ownerDocument.defaultView| in the chrome script. If
> that doesn't work, you can use Components.classes to get the window watcher
> service, from which you can get the most recent navigator window and go from
> there.

Okay, great.  That worked.  Here's what I have now:

https://pastebin.mozilla.org/8869597

And it gives an error on line 82 that contextMenu.ownerDocument.getElementById("spell-check-dictionary-en-US") is null.  Any ideas?  Sorry for all the requests for hand-holding, I just have no idea what all these chrome things actually are.  :(

(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #8)
> > * test_bug569988.html: With e10s, onPromptFocus() isn't called, test hangs. 
> > Without e10s, 'Error: Permission denied to access property "Dialog"'
> 
> I'm surprised that we call onPromptLoad. Does moving the
> onPromptLoad/onPromptFocus stuff into a chrome script help?

Er, we don't call onPromptLoad either, actually.  Moving it into a chrome script seems to work, but how do I call synthesizeKey on gPromptInput.ownerDocument.defaultView?  The function isn't defined in the chrome script, but the window can't be passed back to the main script.
Flags: needinfo?(mrbkap)
Analysis of .xul tests:

editor/composer/test/test_bug434998.xul is meant to test specifically <xul:editor>.

In editor/libeditor/tests/:

* test_bug489202.xul: Deliberately tests APP_TYPE_EDITOR.  I don't think it can be ported.
* test_bug490879.xul: Looks like it should be ported, but I don't know how to execute cmd_copyImageContents without .commandDispatcher.  It would be fine to inject image data into the clipboard in any other way, too.  I tried for a while before giving up.
* test_bug599983.xul: Sets flags on the editor object (allowInteraction/mailMask).  I don't think it can be ported.
* test_bug607584.xul: There's already an HTML test case as well, so this can probably be left XUL.
* test_bug616590.xul: editorElement.makeEditable("htmlmail", true) sounds like it's not portable.
* test_bug636465.xul: Should be ported.
* test_bug646194.xul: Should presumably be ported, although this will make it test for the original crash only on Mac unless I can run commands like cmd_wordPrevious.
* test_bug780908.xul: Not sure.  The original bug was a Thunderbird crash, with no STR for Firefox that I see, so it's probably okay to leave it.
* test_bug1140617: Should be ported.  It seems to fail when I do, though (bug 1140617 comment 59).
* test_selection_move_commands.xul: Definitely looks like it should be ported, but I need a way to run all the commands without commandDispatcher.

Blake, do you know of any way to run commands from plain mochitests, the way files like test_selection_move_commands.xul do using commandDispatcher?  Some of these do seem like we want to convert them to plain tests, but still should use arbitrary internal commands names instead of web APIs, because the web APIs map to the internal commands in a platform-specific way.
(In reply to :Aryeh Gregor (working until May 8) from comment #9)
> https://pastebin.mozilla.org/8869597

It'd be good to start posting patches, I think (browserElement.ownerDocument.defaultView is equivalent to the entire statement you have there :-)).

I'm not sure why you're getting that error. It's possible either that the context menu is getting hidden somehow in the back-and-forth between the chrome script and the page or the document isn't the right one somehow. I'd try maybe not bothering with the back and forth and sending a message once the context menu is shown, you get the state, run the command and hide the popup and sending the state down to the child.

> (In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #8)
> Er, we don't call onPromptLoad either, actually.  Moving it into a chrome
> script seems to work, but how do I call synthesizeKey on
> gPromptInput.ownerDocument.defaultView?  The function isn't defined in the
> chrome script, but the window can't be passed back to the main script.

I wonder if you can import BrowserTestUtils.jsm into the chrome script and use its versions of synthesizeKey?

Ehsan, do you know about the command API (see comment 10, last paragraph).
Flags: needinfo?(mrbkap)
Flags: needinfo?(ehsan)
Flags: needinfo?(ayg)

Comment 12

2 years ago
(In reply to :Aryeh Gregor (working until May 8) from comment #10)
> Blake, do you know of any way to run commands from plain mochitests, the way
> files like test_selection_move_commands.xul do using commandDispatcher? 
> Some of these do seem like we want to convert them to plain tests, but still
> should use arbitrary internal commands names instead of web APIs, because
> the web APIs map to the internal commands in a platform-specific way.

That seems to be fixable by porting the doCommand() function to SpecialPowers.
Flags: needinfo?(ehsan)
Depends on: 1271115
Depends on: 1271119
Depends on: 1271120
No longer depends on: 1271115
Created attachment 8750079 [details] [diff] [review]
0001-Bug-1269209-Fix-incorrect-bug-number-in-test.patch
Flags: needinfo?(ayg)
Attachment #8750079 - Flags: review?(masayuki)
Created attachment 8750081 [details] [diff] [review]
0002-Bug-1269209-DOS-Unix-newlines.patch
Attachment #8750081 - Flags: review?(masayuki)
Created attachment 8750082 [details] [diff] [review]
0003-Bug-1269209-Port-some-editor-mochitests-from-chrome-.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4d16d7cbc89
Attachment #8750082 - Flags: review?(masayuki)
Un-assigning from myself now because I won't be working until August 15.  Someone else should take over landing these if we want them landed before then.
Assignee: ayg → nobody
Status: ASSIGNED → NEW
Depends on: 1271125
Created attachment 8750095 [details] [diff] [review]
0003-Bug-1269209-Port-some-editor-mochitests-from-chrome-.patch

Punted test_composition_event_created_in_chrome.html to bug 1271120 due to try failures.
Attachment #8750082 - Attachment is obsolete: true
Attachment #8750082 - Flags: review?(masayuki)
Attachment #8750095 - Flags: review?(masayuki)
Attachment #8750079 - Flags: review?(masayuki) → review+
Comment on attachment 8750079 [details] [diff] [review]
0001-Bug-1269209-Fix-incorrect-bug-number-in-test.patch

Oops, you don't change the href attribute value!
Attachment #8750079 - Flags: review+ → review-
Comment on attachment 8750081 [details] [diff] [review]
0002-Bug-1269209-DOS-Unix-newlines.patch

Hmm, such change can be reviewed really easier if you use MozReview since it shows differences in each line and wrong linebreaker as red boxes.

I assume that you don't change any line except the linebreaker. If you changed something logically, please create a separate patch for the change.
Attachment #8750081 - Flags: review?(masayuki) → review+
Comment on attachment 8750095 [details] [diff] [review]
0003-Bug-1269209-Port-some-editor-mochitests-from-chrome-.patch

Hmm, could you separate these changes per each test? It's difficult to review all of them once and manage the state of each review state. So, this is too big for review and not necessary to be one patch.
Attachment #8750095 - Flags: review?(masayuki)
Oh, you'll be back August?? Hmm...
Sorry, I planned to figure out how to use MozReview, but I didn't get around to it yet.  I'll try to learn how to use it before I submit future patches for your review.  (Yes, the second patch is just dos2unix, that's why I put it in a separate patch.)

Updated

2 years ago
tracking-e10s: ? → +
Assignee: nobody → ayg
Status: NEW → ASSIGNED
This is now 24 patches, so I'll see if bug 1295987 gets resolved quickly so I can use MozReview to submit them instead of attaching them here.
Green try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be4c26377094&selectedJob=26151345&exclusion_profile=false
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 49

a year ago
mozreview-review
Comment on attachment 8804664 [details]
Bug 1269209 - Fix incorrect bug number in test;

https://reviewboard.mozilla.org/r/88572/#review87892
Attachment #8804664 - Flags: review?(masayuki) → review+

Comment 50

a year ago
mozreview-review
Comment on attachment 8804687 [details]
Bug 1269209 - .ini cleanup;

https://reviewboard.mozilla.org/r/88618/#review87894
Attachment #8804687 - Flags: review?(masayuki) → review+

Comment 51

a year ago
mozreview-review
Comment on attachment 8804665 [details]
Bug 1269209 - DOS -> Unix newlines;

https://reviewboard.mozilla.org/r/88574/#review87896

Perhaps, due to the bug of MozReview, I cannot check where are \r\n and if all of them are replaced correctly.

But the job is enough simple, so, I believe you do right thing.
Attachment #8804665 - Flags: review?(masayuki) → review+

Comment 52

a year ago
mozreview-review
Comment on attachment 8804666 [details]
Bug 1269209 - Port test_async_UpdateCurrentDictionary.html from chrome to plain;

https://reviewboard.mozilla.org/r/88576/#review87904
Attachment #8804666 - Flags: review?(masayuki) → review+

Comment 53

a year ago
mozreview-review
Comment on attachment 8804667 [details]
Bug 1269209 - Port test_bug338427.html from chrome to plain;

https://reviewboard.mozilla.org/r/88578/#review87906

::: editor/composer/test/test_bug338427.html:27
(Diff revision 1)
>  function init() {
> -    Components.utils.import("resource://gre/modules/AsyncSpellCheckTestHelper.jsm");
> -
> +    var onSpellCheck =
> +      SpecialPowers.Cu.import("resource://gre/modules/AsyncSpellCheckTestHelper.jsm")
> +                   .onSpellCheck;
>      var textarea = document.getElementById("editor");
> -    var editor = textarea.editor;
> +    var editor = SpecialPowers.wrap(textarea).editor;

I wonder, we could add SpecialPowers.getEditor() for this, but not the scope of this bug.
Attachment #8804667 - Flags: review?(masayuki) → review+

Comment 54

a year ago
mozreview-review
Comment on attachment 8804668 [details]
Bug 1269209 - Port test_bug678842.html from chrome to plain;

https://reviewboard.mozilla.org/r/88580/#review87908
Attachment #8804668 - Flags: review?(masayuki) → review+

Comment 55

a year ago
mozreview-review
Comment on attachment 8804669 [details]
Bug 1269209 - Port test_bug697981.html from chrome to plain;

https://reviewboard.mozilla.org/r/88582/#review87910

::: editor/composer/test/test_bug697981.html:36
(Diff revision 1)
> +               .onSpellCheck;
> +
>  /** Test for Bug 697981 **/
>  SimpleTest.waitForExplicitFinish();
>  SimpleTest.waitForFocus(function() {
> -  Components.utils.import("resource://gre/modules/AsyncSpellCheckTestHelper.jsm");
> +  script = SpecialPowers.loadChromeScript(function() {

Don't we declare the script as global variable above?
Attachment #8804669 - Flags: review?(masayuki) → review+

Comment 56

a year ago
mozreview-review
Comment on attachment 8804670 [details]
Bug 1269209 - Port test_bug717433.html from chrome to plain;

https://reviewboard.mozilla.org/r/88584/#review87912
Attachment #8804670 - Flags: review?(masayuki) → review+

Comment 57

a year ago
mozreview-review
Comment on attachment 8804671 [details]
Bug 1269209 - Port test_bug1204147.html from chrome to plain;

https://reviewboard.mozilla.org/r/88586/#review87914
Attachment #8804671 - Flags: review?(masayuki) → review+

Comment 58

a year ago
mozreview-review
Comment on attachment 8804672 [details]
Bug 1269209 - Port test_bug1200533.html from chrome to plain;

https://reviewboard.mozilla.org/r/88588/#review87916
Attachment #8804672 - Flags: review?(masayuki) → review+

Comment 59

a year ago
mozreview-review
Comment on attachment 8804673 [details]
Bug 1269209 - Port test_bug1205983.html from chrome to plain;

https://reviewboard.mozilla.org/r/88590/#review87918
Attachment #8804673 - Flags: review?(masayuki) → review+

Comment 60

a year ago
mozreview-review
Comment on attachment 8804674 [details]
Bug 1269209 - Port test_bug1219928.html from chrome to plain;

https://reviewboard.mozilla.org/r/88592/#review87922

::: editor/composer/test/mochitest.ini:31
(Diff revision 1)
>  [test_bug738440.html]
>  [test_bug1200533.html]
>  [test_bug1204147.html]
>  [test_bug1205983.html]
> +[test_bug1219928.html]
> +skip-if = e10s

I you know, could you add short comment for explaining why this is not testable in e10s mode?

Comment 61

a year ago
mozreview-review
Comment on attachment 8804674 [details]
Bug 1269209 - Port test_bug1219928.html from chrome to plain;

https://reviewboard.mozilla.org/r/88592/#review87924

Comment 62

a year ago
mozreview-review
Comment on attachment 8804674 [details]
Bug 1269209 - Port test_bug1219928.html from chrome to plain;

https://reviewboard.mozilla.org/r/88592/#review87926
Attachment #8804674 - Flags: review?(masayuki) → review+

Comment 63

a year ago
mozreview-review
Comment on attachment 8804675 [details]
Bug 1269209 - Port test_bug46555.html from chrome to plain;

https://reviewboard.mozilla.org/r/88594/#review87928
Attachment #8804675 - Flags: review?(masayuki) → review+

Comment 64

a year ago
mozreview-review
Comment on attachment 8804676 [details]
Bug 1269209 - Port test_bug366682.html from chrome to plain;

https://reviewboard.mozilla.org/r/88596/#review87930
Attachment #8804676 - Flags: review?(masayuki) → review+

Comment 65

a year ago
mozreview-review
Comment on attachment 8804677 [details]
Bug 1269209 - Port test_bug471319.html from chrome to plain;

https://reviewboard.mozilla.org/r/88598/#review87932
Attachment #8804677 - Flags: review?(masayuki) → review+

Comment 66

a year ago
mozreview-review
Comment on attachment 8804678 [details]
Bug 1269209 - Port test_bug483651.html from chrome to plain;

https://reviewboard.mozilla.org/r/88600/#review87934
Attachment #8804678 - Flags: review?(masayuki) → review+

Comment 67

a year ago
mozreview-review
Comment on attachment 8804679 [details]
Bug 1269209 - Port test_bug635636.html from chrome to plain;

https://reviewboard.mozilla.org/r/88602/#review87936
Attachment #8804679 - Flags: review?(masayuki) → review+

Comment 68

a year ago
mozreview-review
Comment on attachment 8804680 [details]
Bug 1269209 - Port test_bug830600.html from chrome to plain;

https://reviewboard.mozilla.org/r/88604/#review87938

::: editor/libeditor/tests/mochitest.ini:157
(Diff revision 1)
>  [test_bug790475.html]
>  [test_bug795785.html]
>  [test_bug796839.html]
> +[test_bug830600.html]
> +subsuite = clipboard
> +skip-if = e10s

If you know, please add short comment for explaining why this test cannot run in e10s mode.
Attachment #8804680 - Flags: review?(masayuki) → review+

Comment 69

a year ago
mozreview-review
Comment on attachment 8804681 [details]
Bug 1269209 - Port test_bug1053048.html from chrome to plain;

https://reviewboard.mozilla.org/r/88606/#review87940
Attachment #8804681 - Flags: review?(masayuki) → review+

Comment 70

a year ago
mozreview-review
Comment on attachment 8804682 [details]
Bug 1269209 - Port test_bug1100966.html from chrome to plain;

https://reviewboard.mozilla.org/r/88608/#review87942
Attachment #8804682 - Flags: review?(masayuki) → review+

Comment 71

a year ago
mozreview-review
Comment on attachment 8804683 [details]
Bug 1269209 - Port test_bug1140105.html from chrome to plain;

https://reviewboard.mozilla.org/r/88610/#review87944
Attachment #8804683 - Flags: review?(masayuki) → review+

Comment 72

a year ago
mozreview-review
Comment on attachment 8804684 [details]
Bug 1269209 - Port test_bug1154791.html from chrome to plain;

https://reviewboard.mozilla.org/r/88612/#review87946
Attachment #8804684 - Flags: review?(masayuki) → review+

Comment 73

a year ago
mozreview-review
Comment on attachment 8804685 [details]
Bug 1269209 - Port test_bug1250010.html from chrome to plain;

https://reviewboard.mozilla.org/r/88614/#review87948
Attachment #8804685 - Flags: review?(masayuki) → review+

Comment 74

a year ago
mozreview-review
Comment on attachment 8804686 [details]
Bug 1269209 - Port test_bug1257363.html from chrome to plain;

https://reviewboard.mozilla.org/r/88616/#review87950

Thank you for your great work!!
Attachment #8804686 - Flags: review?(masayuki) → review+
(Assignee)

Comment 75

a year ago
mozreview-review-reply
Comment on attachment 8804669 [details]
Bug 1269209 - Port test_bug697981.html from chrome to plain;

https://reviewboard.mozilla.org/r/88582/#review87910

> Don't we declare the script as global variable above?

I assume you meant to say "Shouldn't" instead of "Don't" here.  You're right, I forgot to declare it.
(Assignee)

Comment 76

a year ago
mozreview-review-reply
Comment on attachment 8804680 [details]
Bug 1269209 - Port test_bug830600.html from chrome to plain;

https://reviewboard.mozilla.org/r/88604/#review87938

> If you know, please add short comment for explaining why this test cannot run in e10s mode.

mrbkap said he didn't know without debugging it (comment #8 in bug).  I changed the commit message to mention it, but I don't think it's useful to add a comment to the .ini file.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8804674 - Attachment is obsolete: true

Comment 100

a year ago
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/autoland/rev/e71532f2babd
Fix incorrect bug number in test; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/816ee7cab76b
DOS -> Unix newlines; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/c7d8d25e09b6
Port test_async_UpdateCurrentDictionary.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/5d2f4867ec7f
Port test_bug338427.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/56e0713ad54b
Port test_bug678842.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/19eb42e6b3eb
Port test_bug697981.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/dda3cfcd2b5c
Port test_bug717433.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/2ebb8eab3834
Port test_bug1204147.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/5f63100d5531
Port test_bug1200533.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/104c0aa10747
Port test_bug1205983.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/cea0421beffa
Port test_bug46555.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/ecf28a55600f
Port test_bug366682.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/a3d0e9ed7d85
Port test_bug471319.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/b74022c44df0
Port test_bug483651.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/016b31e0631f
Port test_bug635636.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/bc025cccc7fb
Port test_bug830600.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/dc9d739edd0f
Port test_bug1053048.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/1b9719af21a2
Port test_bug1100966.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/d6701dc311d0
Port test_bug1140105.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/e77b8468e140
Port test_bug1154791.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/faf627174b48
Port test_bug1250010.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/eac4a21c91f3
Port test_bug1257363.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/0e23de3de2ed
ini cleanup; r=masayuki
I had to back these out because some of the tests are failing:

https://treeherder.mozilla.org/logviewer.html#?job_id=5759323&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=5750710&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/4bf929bdbc4f7bb4a619c41aa849df6a7204e16b
Flags: needinfo?(ayg)

Comment 102

a year ago
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/24e280ad35f7
Backed out changeset 0e23de3de2ed 
https://hg.mozilla.org/mozilla-central/rev/91775ec640ff
Backed out changeset eac4a21c91f3 
https://hg.mozilla.org/mozilla-central/rev/7952763d4b81
Backed out changeset faf627174b48 
https://hg.mozilla.org/mozilla-central/rev/6a42178adcf3
Backed out changeset e77b8468e140 
https://hg.mozilla.org/mozilla-central/rev/81dbe8b3c629
Backed out changeset d6701dc311d0 
https://hg.mozilla.org/mozilla-central/rev/1879e501620f
Backed out changeset 1b9719af21a2 
https://hg.mozilla.org/mozilla-central/rev/bf99c4d5d8e0
Backed out changeset dc9d739edd0f 
https://hg.mozilla.org/mozilla-central/rev/177bf2e81f96
Backed out changeset bc025cccc7fb 
https://hg.mozilla.org/mozilla-central/rev/8abfb7d5845c
Backed out changeset 016b31e0631f 
https://hg.mozilla.org/mozilla-central/rev/d801630ad28c
Backed out changeset b74022c44df0 
https://hg.mozilla.org/mozilla-central/rev/e635799884cc
Backed out changeset a3d0e9ed7d85 
https://hg.mozilla.org/mozilla-central/rev/d1e5c08797c2
Backed out changeset ecf28a55600f 
https://hg.mozilla.org/mozilla-central/rev/426fb19f0b82
Backed out changeset cea0421beffa 
https://hg.mozilla.org/mozilla-central/rev/53bb4e191630
Backed out changeset 104c0aa10747 
https://hg.mozilla.org/mozilla-central/rev/09e1379801c1
Backed out changeset 5f63100d5531 
https://hg.mozilla.org/mozilla-central/rev/4d44d784fa15
Backed out changeset 2ebb8eab3834 
https://hg.mozilla.org/mozilla-central/rev/83b80367c29c
Backed out changeset dda3cfcd2b5c 
https://hg.mozilla.org/mozilla-central/rev/423f556eda8c
Backed out changeset 19eb42e6b3eb 
https://hg.mozilla.org/mozilla-central/rev/4eefe302ced5
Backed out changeset 56e0713ad54b 
https://hg.mozilla.org/mozilla-central/rev/db812cf6198f
Backed out changeset 5d2f4867ec7f 
https://hg.mozilla.org/mozilla-central/rev/efc1aa9ba95b
Backed out changeset c7d8d25e09b6 
https://hg.mozilla.org/mozilla-central/rev/cd8a8b0d7415
Backed out changeset 816ee7cab76b 
https://hg.mozilla.org/mozilla-central/rev/10a2b6ebcd44
Backed out changeset e71532f2babd for various test failures. r=backout a=backout
Oops, I didn't realize (or forgot?) that I was enabling these on Android when moving them.  The chrome tests in this directory don't run on Android, but the plain ones do, so they got enabled as a side-effect.  Masayuki, should I just update all the patches to keep the failing tests disabled on Android, or do you want further action (filing bugs, etc.)?
Flags: needinfo?(ayg)
I think it's okay just to disable them. We cannot decide which should be run also on Android. It's up to you if you'll file bugs for each one because they haven't run on Android. If you'll file bugs, we need to find the author of the tests for ccing to corresponding bug.

Comment 105

a year ago
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a381d07b7f1
Fix incorrect bug number in test; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e7f4f0b0ebb
Port test_async_UpdateCurrentDictionary.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ef54328003a
Port test_bug338427.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bc05800d117
Port test_bug678842.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d34eb5052d
Port test_bug697981.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/359323c68816
Port test_bug717433.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fd7c26a3751
Port test_bug1204147.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/0723157e6398
Port test_bug1200533.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/f220cb8f53e7
Port test_bug1205983.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/739b0d4719c5
Port test_bug46555.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8fafe993a33
Port test_bug366682.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e5fe6e88fb4
Port test_bug471319.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/49f889ed60ed
Port test_bug483651.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/50b74197e3c4
Port test_bug635636.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/52a8a55852a8
Port test_bug830600.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaa7cf220e4c
Port test_bug1053048.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2e81e5adf55
Port test_bug1100966.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e54ecf6669c
Port test_bug1140105.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/100dac64a19a
Port test_bug1154791.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/12ef941bdc62
Port test_bug1250010.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6097b269c66
Port test_bug1257363.html from chrome to plain; r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/f474e1e8bec7
ini cleanup; r=masayuki
Greenish try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe56506f5253  The failures are not in files touched by this bug.

Comment 107

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6a381d07b7f1
https://hg.mozilla.org/mozilla-central/rev/8e7f4f0b0ebb
https://hg.mozilla.org/mozilla-central/rev/1ef54328003a
https://hg.mozilla.org/mozilla-central/rev/4bc05800d117
https://hg.mozilla.org/mozilla-central/rev/36d34eb5052d
https://hg.mozilla.org/mozilla-central/rev/359323c68816
https://hg.mozilla.org/mozilla-central/rev/3fd7c26a3751
https://hg.mozilla.org/mozilla-central/rev/0723157e6398
https://hg.mozilla.org/mozilla-central/rev/f220cb8f53e7
https://hg.mozilla.org/mozilla-central/rev/739b0d4719c5
https://hg.mozilla.org/mozilla-central/rev/b8fafe993a33
https://hg.mozilla.org/mozilla-central/rev/2e5fe6e88fb4
https://hg.mozilla.org/mozilla-central/rev/49f889ed60ed
https://hg.mozilla.org/mozilla-central/rev/50b74197e3c4
https://hg.mozilla.org/mozilla-central/rev/52a8a55852a8
https://hg.mozilla.org/mozilla-central/rev/eaa7cf220e4c
https://hg.mozilla.org/mozilla-central/rev/c2e81e5adf55
https://hg.mozilla.org/mozilla-central/rev/3e54ecf6669c
https://hg.mozilla.org/mozilla-central/rev/100dac64a19a
https://hg.mozilla.org/mozilla-central/rev/12ef941bdc62
https://hg.mozilla.org/mozilla-central/rev/e6097b269c66
https://hg.mozilla.org/mozilla-central/rev/f474e1e8bec7
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Congratulations! And thank you for your great work!
You need to log in before you can comment on or make changes to this bug.