686821 - "level" object attribute not present on headings while tab is in background

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Attachment: Test case.

Technical str:
1. Make sure at least 1 tab is open (tab 1).
2. Create a new tab and open the attached test case (tab 2).
3. Retrieve the heading accessible and check the value of the "level" object attribute.
Result (correct): The value is "1".
4. Switch to tab 1.
5. Retrieve the same heading accessible as in step 3 (while the document is still in a background tab) and check the value of the "level" object attribute.
Expected: The value should be "1".
Actual: The attribute is not present.

This causes the following problem for NVDA users:
Str:
0. Start NVDA.
1. Make sure at least 1 tab is open (tab 1).
2. Create a new tab and open the attached test case (tab 2). NVDA should report the document.
3. Quickly (within 5 seconds), switch to tab 1.
4. Wait 5 seconds, then switch to tab 2.
5. Press control+end to move to the bottom line of the document.
Expected: NVDA should say "heading level 1 Test Heading".
Actual: NVDA says "heading Test Heading".

If having the level attribute always present is not possible, an object attribute changed event should be fired on the heading when the level becomes available. This will cause NVDA to re-render the object.

Related NVDA issue ticket: http://www.nvda-project.org/ticket/1714

Comment 1

[James Teh [:Jamie]]

I forgot to add these steps to the STR for the NVDA part:
(In reply to James Teh [:Jamie] from comment #0)
> 5. Press control+end to move to the bottom line of the document.
> Expected: NVDA should say "heading level 1 Test Heading".
> Actual: NVDA says "heading Test Heading".
6. Press NVDA+f5 to completely re-render the document.
7. Press control+end to move to the bottom line of the document.
Result (correct): NVDA says "heading Test Heading".
Reason: The "level" attribute is now present because the document is in the active tab. However, when NVDA first rendered the heading (while the tab was in the background), it wasn't present.

Comment 2

[alexander :surkov (:asurkov)]

We skip group position calculation for invisible accessibles. Should be fixed by bug 591363 if we won't expose invisible for accessible in background tab.

Comment 3

[alexander :surkov (:asurkov)]

David, did your bug 591363 fixed this issue?

Comment 4

[James Teh [:Jamie]]

This bug still exists. If bug 591363 did originally fix it, I guess that part of the fix was backed out.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #4)
> This bug still exists. If bug 591363 did originally fix it, I guess that
> part of the fix was backed out.

bug 686821 was regressed at some point, the state invisible is exposed on heading in this case (that bug doesn't have proper mochitests).

Comment 6

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #4)
> This bug still exists. If bug 591363 did originally fix it, I guess that
> part of the fix was backed out.

I meant bug 591363. The regression was in bug 722248.

Comment 7

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 8

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #7)
> Created attachment 658015 [details] [diff] [review]
> patch

1) patch: expose offscreen state (no invisible state) on background tab accessible (the fix of bug 591363)
2) closeBrowserWindow is uncommented locally
3) style.visibility.hidden tests were removed because the tree mutations of this kind are covered by treeupdate/test_visibility.html tests

Comment 9

[Trevor Saunders (:tbsaunde)]

Comment on attachment 658015 [details] [diff] [review]
patch

Add the bug link to the test?

Also why don't we add tests for the issue the bug reports?  while we're here so that people don't regress it in random ways in the future?

Comment 10

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #9)

> Also why don't we add tests for the issue the bug reports?  while we're here
> so that people don't regress it in random ways in the future?

In general it makes sense. Usually I provide minimum amount of necessary tests. In this case it's hard to regress the same way so I skipped this part.

Comment 11

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28f0e2bbc292

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/28f0e2bbc292