Open Bug 668283 Opened 13 years ago Updated 2 years ago

Make content/ mochitests pass on desktop Fennec

Categories

(Core :: DOM: Core & HTML, defect, P5)

defect

Tracking

()

People

(Reporter: jdm, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(7 files, 10 obsolete files)

(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
This is a dumping ground for my patches that will add SpecialPowers APIs and change tests that require changing.
Depends on: 621363, 621362
Attached patch Make somce of content/base run green (obsolete) (deleted) — Splinter Review
Assignee: nobody → josh
Attached patch Make content/events run to completion (obsolete) (deleted) — Splinter Review
Attached patch Make content/media run to completion (obsolete) (deleted) — Splinter Review
awesome stuff. Is this making them green, or just able to run to completion?
Attached patch Make content/svg run to completion (obsolete) (deleted) — Splinter Review
Attached patch Make content/xbl run to completion (obsolete) (deleted) — Splinter Review
Attached patch Make content/xul run to completion (obsolete) (deleted) — Splinter Review
With the patches from the dependent bugs, this makes all patches run green for me (as far as I can remember). However, there are a couple pieces of tests disabled due to problems that can't be avoided with the SpecialPowers fairy dust: a content/base test exposed a necko bug we need to address (it's commented out in order to run to completion), and there were some problems with waitForFocus and synthesizeStuff that I recall.
are these tests ones that should be run as mochitest-chrome instead? the synthesize stuff is a little tricky and the waitForFocus is really tricky. the general mindset for getting tests running on mobile is 'something running is better than nothing running'. Keeping to that motto, getting the 90% of the tests that are easier to fix and get running checked in will be a huge win.
Comment on attachment 542891 [details] [diff] [review] Make content/media run to completion Review of attachment 542891 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/specialpowers/content/specialpowers.js @@ +441,1 @@ > }, would you have a problem making this a bit more generic: getDir: function(type) { if (type in ['CurWorkD', 'tmpD', 'ProfD']) return sendSyncMessage('SPDirSvc', {'type': type})[0] + '/'; } Also, do we need to send a sync message to get this information, or can we do it all locally?
Comment on attachment 542890 [details] [diff] [review] Make content/events run to completion Review of attachment 542890 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/test/test_bug331959.html @@ +39,2 @@ > function doNextTest() { > +alert("woo"); some debug stuff in this patch @@ +142,5 @@ > "Return on submit button inside an anchor should submit the form"); > button.focus(); > synthesizeKey("VK_RETURN", {}, testWin); > } > +alert("woo?"); more debug stuff ::: content/events/test/test_bug350471.xul @@ +238,5 @@ > prefSvc.clearUserPref("mousewheel.system_scroll_override_on_root_content.enabled"); > } > > window.onload = function () { > + //netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); why comment out this line, we can just remove it. @@ +250,4 @@ > clearPrefs(); > win.close(); > SimpleTest.finish(); > + //}); why is this commented out? ::: testing/mochitest/tests/SimpleTest/EventUtils.js @@ +239,5 @@ > * aWindow is optional, and defaults to the current window object. > */ > function synthesizeMouseScroll(aTarget, aOffsetX, aOffsetY, aEvent, aWindow) > { > + //netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect'); we can remove this. ::: testing/mochitest/tests/SimpleTest/SimpleTest.js @@ +313,5 @@ > if (!targetWindow) > targetWindow = window; > > if (ipcMode) { > + var domutils = SpecialPowers.DOMWindowUtils; I have a patch that changes this to pass in a window and get the utils for that window, otherwise this is cool!
Comment on attachment 542889 [details] [diff] [review] Make somce of content/base run green Review of attachment 542889 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/test/test_CrossSiteXHR_origin.html @@ +46,2 @@ > file: 'http://example.org/tests/content/base/test/file_CrossSiteXHR_inner_data.sjs' > + },*/ why is this commented out? ::: content/base/test/test_XHRSendData.html @@ +37,1 @@ > can we remove this? @@ +55,5 @@ > function createFileWithDataExt(fileData, extension) { > + var basePath = SpecialPowers.getProfileDir(); > + basePath += "testfile" + extension; > + SpecialPowers.writeFile(basePath, fileData); > + return basePath; I am trying to figure out the best way to approach fileIO with special powers. we have a handful of different needs, but it can be generalized a bit. Overall, I like this approach, I just need to chew on it some more ::: content/base/test/test_bug331959.html @@ +37,2 @@ > testInputInLinkMouse, testInputInLinkKeyboard, > + testButtonInLinkMouse, testButtonInLinkKeyboard*/ ]; why are these commented out? ::: testing/mochitest/runtests.py @@ +519,1 @@ > hmm, is this what we want here? I would like to get some consensus on this before calling it good.
(In reply to comment #10) > Comment on attachment 542891 [details] [diff] [review] [review] > Make content/media run to completion > > Review of attachment 542891 [details] [diff] [review] [review]: > ----------------------------------------------------------------- > > ::: testing/mochitest/specialpowers/content/specialpowers.js > @@ +441,1 @@ > > }, > > would you have a problem making this a bit more generic: > getDir: function(type) { > if (type in ['CurWorkD', 'tmpD', 'ProfD']) > return sendSyncMessage('SPDirSvc', {'type': type})[0] + '/'; > } > > > Also, do we need to send a sync message to get this information, or can we > do it all locally? There's no profile in the content process, so we need to get this information from the parent. And yeah, I'm fine with generalizing.
(In reply to comment #12) > ::: content/base/test/test_CrossSiteXHR_origin.html > @@ +46,2 @@ > > file: 'http://example.org/tests/content/base/test/file_CrossSiteXHR_inner_data.sjs' > > + },*/ > > why is this commented out? Because it exposes bug 661604 and causes the test to time out. > ::: content/base/test/test_XHRSendData.html > @@ +37,1 @@ > > > > can we remove this? Not entirely sure what you're referring to. > ::: content/base/test/test_bug331959.html > @@ +37,2 @@ > > testInputInLinkMouse, testInputInLinkKeyboard, > > + testButtonInLinkMouse, testButtonInLinkKeyboard*/ ]; > > why are these commented out? Pretty sure they required syntheSizeFoo calls which weren't working. > ::: testing/mochitest/runtests.py > @@ +519,1 @@ > > > > hmm, is this what we want here? I would like to get some consensus on this > before calling it good. Oh, definitely not. That was for my own sanity because there are so many assertions in Fennec that the stack traces were overwhelming me.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Depends on: 669544
Comment on attachment 542889 [details] [diff] [review] Make somce of content/base run green I was wrong, this patch gets no where near to finishing content/base. I'm plugging away at it now.
Attachment #542889 - Attachment description: Make content/base run to completion → Make somce of content/base run green
Depends on: 670030
With all dependent bugs fixed, content/base is completely green (except for one mysterious failure in the onunload handler of test_plugin_freezing.html)
I don't see tests moved to mochitest-chrome in this patch? we need to fix the waitForFocus and synthesize* functions to work in specialpowers. I have some patches for the synthesizeMouse/Key functions and they seem to work, but I haven't tackled waitForFocus.
Blocks: 552523
Blocks: 650386
Depends on: 675669
Depends on: 649360
Depends on: 678389
Depends on: 678595
Depends on: 679222
Depends on: 670897
Depends on: 679791
Depends on: 679833
Depends on: 679841
(In reply to Ian Melven :imelven from comment #19) > *** Bug 674255 has been marked as a duplicate of this bug. *** Hey Josh, the scope of this bug is quite a bit larger than the functionality Ian needs to work on some security bugs, so if it's okay, I'm going to reopen bug 674255 and perhaps you can remove that from this bug's scope.
Yes, that is absolutely the right way to go here.
Depends on: 680275
No longer blocks: 552523
No longer blocks: 650386
Blocks: mochi10s
Depends on: 605365
With my most recent patches, I get these failures: 25948 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug560780.html | got the mousedown events - got 0, expected 12 30663 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug622117.html | Should have loaded link - got "Click me", expected "PASS" 30670 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug622246.html | Should have loaded link - got "Click me", expected "PASS" 35508 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_range_bounds.html | [SimpleTest/SimpleTest.js, window.onerror] - An error occurred: e1.stopWatchingInstanceCount is not a function at http://mochi.test:8888/tests/content/base/test/test_plugin_freezing.html:21
Attached patch Make content/base mochitests pass WIP (obsolete) (deleted) — Splinter Review
Attachment #544640 - Attachment is obsolete: true
Attached patch Make content/events mochitests pass WIP (obsolete) (deleted) — Splinter Review
Attachment #556980 - Attachment is obsolete: true
Attached patch Make content/events mochitests pass WIP (obsolete) (deleted) — Splinter Review
Attachment #556981 - Attachment is obsolete: true
Attachment #542889 - Attachment is obsolete: true
Attachment #542890 - Attachment is obsolete: true
Attachment #556986 - Attachment is obsolete: true
I'm not sure if setting all these preload attributes is the right fix here. It feels like there should be a pref controlling this or something.
Attached patch Make content/svg pass WIP (deleted) — Splinter Review
Attachment #557206 - Attachment description: Make content/events tests pass WIP → Make content/media tests pass WIP
Attached patch Make content/xul tests pass WIP (deleted) — Splinter Review
Attachment #542891 - Attachment is obsolete: true
Attachment #542892 - Attachment is obsolete: true
Attachment #542893 - Attachment is obsolete: true
Attachment #542895 - Attachment is obsolete: true
Mark, the patches here are likely quite bitrotted, but may help your e10s test efforts.
Not working on this.
Assignee: josh → nobody
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: