1700362 - DocAccessible::IsLoadEventTarget doesn't behave as intended

Status: NEW

(Core :: Disability Access APIs, task)

Description

[James Teh [:Jamie]]

IsLoadEventTarget is supposed to return false for iframe documents which are still loading, thus preventing doc load complete events from firing in this case. This was done many years ago because it apparently caused problems for screen readers:

  // b) document load event on sub documents causes screen readers to act is if
  // entire page is reloaded.

However, this has been broken since e10s; i.e. we do fire doc load complete events for iframes, even if the parent doc is still loading. The reason is that IsLoadEventTarget checks if the parent nsIDocShellTreeItem is the root and assumes this means the document is a tab (top level) document. In the parent process, this is true because the root is always the chrome. However, in content processes, the root is the top level content document! That means we treat an iframe below a top level document as if it were a top level document.

I can fix this, but I think we should instead remove this "intended" behaviour and return true for iframes regardless of the parent's loading state, keeping the current "unintentional" behaviour in content processes:

1. Our browser tests rely on doc load complete events from iframes.
2. Mac wants AXLayoutComplete to be fired on iframes and we map that from doc load complete.
3. We've been doing this since e10s (3+ years) and it doesn't seem to have hurt any screen readers, so presumably, this has been fixed in screen readers.
4. This check can never work for OOP (Fission) iframes, since the parent document is in a different process and we thus can't query its loading state.

In short, this would just remove dead code so that it reflects what happens in reality. It does change the behaviour for iframes in the parent process, but those are pretty rare and we should really be consistent anyway between parent and content.

Comment 1

[James Teh [:Jamie]]

Chrome seems inconsistent here. It often fires doc load complete events for iframes, but it sometimes doesn't.

Comment 2

[James Teh [:Jamie]]

(In reply to James Teh [:Jamie] from comment #0)

1. Our browser tests rely on doc load complete events from iframes.

Actually, this is only true for Fission iframes. So, reverting to the originally intended behaviour wouldn't break our tests... but I still think we should be consistent here.

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1700362: Don't check whether the parent DocAccessible is loading when determining whether to fire doc load events.

This code has been broken in content processes since e10s; i.e. we currently fire doc load complete events for iframes, even if the parent doc is still loading.
Rather than returning to the pre-e10s behaviour, this check has been removed instead for several reasons:

1. Mac wants AXLayoutComplete to be fired on iframes and we map that from doc load complete.
2. We've been doing this since e10s (3+ years) and it doesn't seem to have hurt any screen readers, so presumably, this has been fixed in screen readers.
3. This check can never work for OOP (Fission) iframes, since the parent document is in a different process and we thus can't query its loading state.
4. The behaviour is currently inconsistent between parent process documents and content process documents. Removing this check makes it consistent.

Comment 4

[James Teh [:Jamie]]

(In reply to James Teh [:Jamie] from comment #0)

4. This check can never work for OOP (Fission) iframes, since the parent document is in a different process and we thus can't query its loading state.

Blocking a11y-fission for this reason. However, note that this is purely about code correctness; the actual behaviour (despite the confusing code) is already what we want for Fission.

Comment 5

[Neha Kochar [:neha]]

Marking it for Fission Future since the code is behaving correctly for Fission per comment 4.