Closed
Bug 1198163
Opened 9 years ago
Closed 9 years ago
Enable browserElement_SetInputMethodActive.js in emulator
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(firefox45 fixed)
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: timdream, Assigned: timdream)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
timdream
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Updated•9 years ago
|
Comment hidden (obsolete) |
Assignee | ||
Comment 2•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Component: Mochitest Chrome → Mochitest
Assignee | ||
Comment 3•9 years ago
|
||
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.
Comment 4•9 years ago
|
||
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.
Assignee | ||
Comment 5•9 years ago
|
||
(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)
Comment 6•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•9 years ago
|
||
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
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8678044 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
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
Assignee | ||
Comment 11•9 years ago
|
||
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.
Assignee | ||
Comment 12•9 years ago
|
||
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 13•9 years ago
|
||
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+
Updated•9 years ago
|
Summary: SpecialPowers.pushPermissions() does not work in b2g mochitests (runs on content process) → Enable browserElement_SetInputMethodActive.js in emulator
Assignee | ||
Comment 14•9 years ago
|
||
B2G ICS m-2 re-triggered 5 times to ensure the test is stable.
Updated•9 years ago
|
Attachment #8684733 -
Flags: feedback+ → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 15•9 years ago
|
||
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
Assignee | ||
Comment 16•9 years ago
|
||
Rebased.
Attachment #8684733 -
Attachment is obsolete: true
Flags: needinfo?(timdream)
Attachment #8685833 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 17•9 years ago
|
||
Keywords: checkin-needed
Comment 18•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•