1845407 - Firefox does not initially provide accessible API on launch.

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Eitan Isaacson [:eeejay]]

If we set up an external listener to wait for Firefox to launch and then query its accessibility API, we fail. Accessibility services such as Voice Control do this and cannot access Firefox for this reason. Below is a python script that illustrates the issue. It should ideally retrieve the role and name of the top-level application accessibility object.

from Foundation import NSObject
from AppKit import NSWorkspaceDidLaunchApplicationNotification, NSWorkspace
from ApplicationServices import AXUIElementCreateApplication, AXUIElementCopyAttributeValue
from Quartz import CFRunLoopRun

class Observer(NSObject):
  def run_(self, notification):
    info = notification.userInfo()
    print("LAUNCH %s (%s)" %
          (info["NSApplicationName"], info["NSApplicationBundleIdentifier"]))
    pid = notification.userInfo()['NSApplicationProcessIdentifier'];
    acc = AXUIElementCreateApplication(pid)
    err, name = AXUIElementCopyAttributeValue(acc, "AXTitle", None)
    assert err == 0, "Error getting accessible's AXTitle"
    err, role= AXUIElementCopyAttributeValue(acc, "AXRole", None)
    assert err == 0, "Error getting accessible's AXRole"
    print("name=%s role=%s" % (name, role))
    exit(0)


observer = Observer.new()
NSWorkspace.sharedWorkspace().notificationCenter().addObserver_selector_name_object_(
  observer, "run:", NSWorkspaceDidLaunchApplicationNotification, None)
CFRunLoopRun()

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1845407 - Don't pump apple events before application starts. r?nika!

When we call nextEventMatchingMask we send a "did launch app" notification to the OS workspace. If we do that before we call [NSApp run] accessibility isn't hooked up yet internally in AppKit and we fail to respond to a11y API queries.

Pumping Apple events is currently used for getting url requests in order to launch Firefox with the URLs loading correctly in a browser window. We can achieve the same result by using application:openURLs: in the delegate and calling run on the NSApp earlier and then stopping the loop in order to build the command line and finish initializing Firefox.

Also pumping the apple event queue should be deferred in ProcessNextNativeEvent until [NSApp run] is called.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c869f080aad6
Don't pump apple events before application starts. r=nika

Comment 4

[Cristian Tuns]

Backed out for causing multiple failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-ERROR | netwerk/test/marionette/test_purge_http_cache_at_shutdown.py PurgeHTTPCacheAtShutdownTestCase.test_asetup | OSError: Process has been unexpectedly closed (Exit code: 0) (Reason: Process still running 70.0s after quit request)
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | toolkit/components/backgroundtasks/tests/browser/browser_backgroundtask_specific_pref.js | Test timed out -
• Failure Log
• Failure line: TEST-UNEXPECTED-TIMEOUT | browser/base/content/test/sanitize/browser_sanitizeDialog.js (finished) | application timed out after 370 seconds with no output

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d85342952298
Backed out changeset c869f080aad6 for causing marionette failures in test_purge_http_cache_at_shutdown.py  CLOSED TREE

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4bfe39c76f0
Don't pump apple events before application starts. r=nika

Comment 7

[Sandor Molnar[:smolnar]]

Backed out for causing Gtest failures at TestNrSocketTest.FullConeTimeout

Backout link: https://hg.mozilla.org/integration/autoland/rev/65da50253a19b9adc6a900d4b221bda30156b885

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | TestNrSocketTest.FullConeTimeout | Value of: CheckConnectivityVia(public_addrs_[0], private_addrs_[0], sender_external_address)

Comment 8

[Eitan Isaacson [:eeejay]]

These unit test failures seem arbitrary and I don't think are related. I tested those unit tests locally and they passed. Same with try.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04750ca05094
Don't pump apple events before application starts. r=nika

Comment 10

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on mozapps/update/

• backout: https://hg.mozilla.org/integration/autoland/rev/124fe9b28a55a1a17a379f73bf49e0c6a1337d2f
• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=04750ca05094f9b3d88cbe960779afa9ca29e1c4&selectedTaskRun=IeRpO_SjS9a9mW6fs6wdOg.0
• failure log: https://treeherder.mozilla.org/logviewer?job_id=431537621&repo=autoland&lineNumber=4637

[task 2023-10-06T03:37:13.795Z] 03:37:13     INFO -  TEST-START | toolkit/mozapps/update/tests/unit_base_updater/marMissingUpdateSettingsStage.js
[task 2023-10-06T03:42:14.015Z] 03:42:14  WARNING -  TEST-UNEXPECTED-TIMEOUT | toolkit/mozapps/update/tests/unit_base_updater/marMissingUpdateSettingsStage.js | Test timed out
[task 2023-10-06T03:42:14.016Z] 03:42:14     INFO -  TEST-INFO took 300219ms
[task 2023-10-06T03:42:14.016Z] 03:42:14     INFO -  >>>>>>>
[task 2023-10-06T03:42:14.016Z] 03:42:14     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2023-10-06T03:42:14.016Z] 03:42:14     INFO -  "03:37:13:927 | TEST-INFO | xpcshellUtilsAUS.js | [setupTestCommon : 836] start - general test setup"
[task 2023-10-06T03:42:14.016Z] 03:42:14     INFO -  TEST-PASS | toolkit/mozapps/update/tests/unit_base_updater/marMissingUpdateSettingsStage.js | setupTestCommon - [setupTestCommon : 838] gTestID should be 'undefined' (setupTestCommon should only be called once) - "undefined" === "undefined"
[task 2023-10-06T03:42:14.017Z] 03:42:14     INFO -  (xpcshell/head.js) | test pending (2)

Comment 11

[Stephen A Pohl [:spohl]]

These test failures are looking very scary, as they involve updates (and possibly the failure to relaunch after updates, or worse). We may want to get QA to confirm that this all still works properly. n-i=bytesized for awareness and opinion.

Comment 12

[Eitan Isaacson [:eeejay]]

This is puzzling because the failing tests are supposed to be skipped in mac.

https://searchfox.org/mozilla-central/source/toolkit/mozapps/update/tests/unit_base_updater/xpcshell.ini#54

Comment 13

[Eitan Isaacson [:eeejay]]

oh. i misread the ini file. the options don't fall through in that way.

Comment 14

[Robin Steuber (she/her) [:bytesized]]

At first glance, these test failures are confusing. They time out after seeming to have done everything successfully and having printed "calling doTestFinish" and "test finished". I'm not entirely clear on what's going on here. I'm going to keep the needinfo on me because I want to take another look at this in a bit.

(In reply to Stephen A Pohl [:spohl] from comment #11)

and possibly the failure to relaunch after updates, or worse

Are you suggesting this because of the nature of the patch, or for some other reason? I'm having a bit of a hard time understanding the patch, so I can't tell if it is likely to cause this sort of problem.

Comment 15

[Robin Steuber (she/her) [:bytesized]]

whoops, meant to leave that.

Comment 16

[Eitan Isaacson [:eeejay]]

(In reply to Robin Steuber (they/them) [:bytesized] from comment #14)

Are you suggesting this because of the nature of the patch, or for some other reason? I'm having a bit of a hard time understanding the patch, so I can't tell if it is likely to cause this sort of problem.

Because this patch touches app launch there is a possibility that this affects relaunch after update, which is what might be failing as you suggest?

I am having trouble running these xpc tests locally, I get a system error dialog "dir" is damaged and can't be opened. You should move it to the Trash. Any pointers on how to run this locally are welcome!

Comment 17

[Robin Steuber (she/her) [:bytesized]]

I'm afraid that I wasn't able to make any progress figuring out why this test is failing, but I was able to figure out the issue that I believe you are having running the test locally. I believe that you need to add ac_add_options --enable-unverified-updates to your mozconfig and rebuild.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/374f05be6d50
Don't pump apple events before application starts. r=nika

Comment 19

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/374f05be6d50

Comment 20

[Norisz Fay [:noriszfay]]

Backed out for causing bc failures on browser/components/* , browser/base/content/*

Backout link

Push with failures

Failure log

Comment 21

[Julien Cristau [:jcristau]]

The backout is on central now too:
https://hg.mozilla.org/mozilla-central/rev/9fe5bdf354caa00d21aab24def8841ee4b2abdab

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/275d06a26aff
Don't pump apple events before application starts. r=nika

Comment 23

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/275d06a26aff

Comment 24

[Camelia Badau [:cbadau], Desktop Test Engineering]

Hello!
Is manual testing required here? If yes, can you provide us some steps to reproduce?