Closed Bug 1198163 Opened 9 years ago Closed 9 years ago

Enable browserElement_SetInputMethodActive.js in emulator

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: timdream, Assigned: timdream)

References

Details

Attachments

(1 file, 4 obsolete files)

Please read bug 974770 comment 22. The same SpecialPowers.pushPermissions() works on Fx Desktop but not ICS mochitests. The special power code does wait for the permission here [1] but the tests still fails at "Cannot access mozInputMethod.". [1] https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-unknown-linux-gnu/_tests/reftest/specialpowers/chrome/specialpowers/content/specialpowersAPI.js#858 This may be a timing issue since emulator is very slow. The only other difference I could think of is because of the page is running on content process, so bug 1094055 might be related (or not).
Please disregard comment 1.
Component: Mochitest → Mochitest Chrome
Summary: SpecialPowers.pushPermissions() only works in b2g mochitests with a setTimeout after it → SpecialPowers.pushPermissions() does not work in b2g mochitests (runs on content process)
Component: Mochitest Chrome → Mochitest
I tried to convert this line SpecialPowers.addPermission('input-manage', true, document); to pushPermissions too but it won't work either. Also if the input-manage permission failed, the assertion 'Can access setInputMethodActive.' should fail before that one, but it didn't, indicating the parent frame does have the input-manage permission, but the child frame doesn't have the input permission set.
It's weird if pushPermissions doesn't work on the emulator because pushPermissons is used very widely in other tests. Many times I found the permission setting didn't work because the wrong context was set. Maybe you could check that.
(In reply to Kan-Ru Chen [:kanru] from comment #4) > It's weird if pushPermissions doesn't work on the emulator because > pushPermissons is used very widely in other tests. Many times I found the > permission setting didn't work because the wrong context was set. Maybe you > could check that. Is that even possible if the same test works on Firefox desktop? Do I need to pass different context when calling from content process?
Flags: needinfo?(kchen)
The difference is that on emulator the test runs inside a Mochitest app. I suspect the keyboard iframe inherits the appId of the Mochitest app so if you set NO_APP_ID it wouldn't work. To make it work you could either pass iframe.contentDocument as context or add mozapp attribute to the iframe and the manifestURL as context.
Flags: needinfo?(kchen)
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Attached patch bug1198163.patch (obsolete) (deleted) — Splinter Review
Update context per above comment, apply on top of bug 1196654 since the test touches the same thing. Let's see if it works on Emulator. https://treeherder.mozilla.org/#/jobs?repo=try&revision=826d00820fe7
There are two problems with the current patch: 1. Removing a postMessage() between file_inputmethod.html and frame script caused a timing issue only reproducible on Try. Going to use a setTimeout() to workaround it. 2. |frame.contentWindow.document| does not work -- getting Exception like Error: Permission denied to access property "document" at http://mochi.test:8888/tests/dom/browser-element/mochitest/browserElement_SetInputMethodActive.js:80 no matter how I tried to unwrap the object. Since our goal is to workaround NO_APP_ID, I am going to try to put the App Id of Mochitest into there and see if our frames can get the permission that way.
Attached patch bug1198163.patch (obsolete) (deleted) — Splinter Review
This is the current progress on (2). I can make SetInputMethodActive work now, I however cannot make Proxy test cases work yet.
Attachment #8678154 - Attachment is obsolete: true
For the SetInputMethodActive case, :cyu said the mm parent process only considers the permission of the root frame because cpmm is a service and it is always shared in the entire process. For the Proxy case, it is unclear what BrowserElementParent loaded inside the content process considers.
Attached patch bug1198163.patch (obsolete) (deleted) — Splinter Review
After talking to :kanru last week it is determined that the MessageManager#assertPermission here [1] will always throw or return false in content process, which is exactly the mochitest env I am trying to run this test on (since bug 1094055 was never fixed). [1] https://hg.mozilla.org/mozilla-central/annotate/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/browser-element/BrowserElementParent.js#l102 This does not affect SetInputMethodActive case because Keyboard.jsm is always loaded at the chrome process by shell.js. Yet, this means both test case will fail at e10s Firefox desktop -- which the entire browser-element test suite is being skipped currently. So we are giving up fixing the Proxy test case here, but update the comment in mochitest.ini accordingly. Someone has to fix bug 1094055 or we would need to "normalize" these chrome/content parent/child relationships support nested oop (bug 1020135) and always put the mozbrowser iframe into a remote process. https://treeherder.mozilla.org/#/jobs?repo=try&revision=88d56ab8d909
Attachment #8678718 - Attachment is obsolete: true
Attachment #8684733 - Flags: review?(kchen)
Comment on attachment 8684733 [details] [diff] [review] bug1198163.patch Review of attachment 8684733 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/browser-element/mochitest/browserElement_SetInputMethodActive.js @@ +92,5 @@ > + context: { > + url: SimpleTest.getTestFileURL('/file_empty.html'), > + originAttributes: { > + appId: currentAppId, > + inBrowser: true inBrowser=true will make this a browser frame. The permission will not apply to the input method frame, I think. @@ +101,5 @@ > + if (inApp) { > + // The current document would also need to be given access for IPC to > + // recognize our permission (why)? > + permissions.push({ > + type: 'input', allow: true, context: document }); I think it's because you set the gInputMethodFrames to use mozapp=currentAppManifestURL so they are equivalent to the Mochitest app. If you set the manifest to http://example.org/manifest.webapp then you probably don't have to set this conditionally.
Attachment #8684733 - Flags: review?(kchen) → feedback+
Summary: SpecialPowers.pushPermissions() does not work in b2g mochitests (runs on content process) → Enable browserElement_SetInputMethodActive.js in emulator
B2G ICS m-2 re-triggered 5 times to ensure the test is stable.
Attachment #8684733 - Flags: feedback+ → review+
Hi, this failed to apply : patching file dom/browser-element/mochitest/mochitest.ini Hunk #1 FAILED at 120 1 out of 2 hunks FAILED -- saving rejects to file dom/browser-element/mochitest/mochitest.ini.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and refresh bug1198163.patch
Flags: needinfo?(timdream)
Keywords: checkin-needed
Attached patch bug1198163.patch (deleted) — Splinter Review
Rebased.
Attachment #8684733 - Attachment is obsolete: true
Flags: needinfo?(timdream)
Attachment #8685833 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: