random fail of image map (area) accessible creation

RESOLVED FIXED in mozilla16

Status

()

Core
Disability Access APIs
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: surkov, Assigned: capella)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla16
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [leave open][mentor=marco.zehe@googlemail.com][lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
It was caught by mochitests (bug 569425). I file new bug to avoid spamming by reports of failing mochitests.

David assumed (bug 569425 comment #10) that the bug might related with lazy frame construction. CC'ing Boris.

Boris, it sounds the real problem should be we can't create image map accessible what happens in GetAreaAccessible - http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessibilityService.cpp#1786. 

On the another hand we could fail early before before GetAreaAccessible is called where we use the following assumption  content->GetPrimaryFrame()->GetContent() != content (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessibilityService.cpp#1438). Can it fail for image map node?
(Reporter)

Updated

7 years ago
Blocks: 569425
(Reporter)

Updated

7 years ago
Summary: can't create image map (area) accessible randomly → random fail of image map (area) accessible creation
I'm fairly certain what the problem is here, bug 569425 comment 11, it is not lazy frame construction, image maps already had their own annoying form of setting primary frame pointer asynchronously.
This bug is now to deal with the issue mentioned in bug 569425 comment 14, where accessible creation will fail if there has not been a mouse event over the image or the image has not been painted.
(Reporter)

Updated

5 years ago
Blocks: 741474
(Reporter)

Comment 3

5 years ago
actually the fix of the bug 741474 is not really connected, for bug 741474 we should mouse over the image map.

for this bug I think we should get rid mouse over hack and just wait for reorder event.
No longer blocks: 741474
(Reporter)

Comment 4

5 years ago
Created attachment 615258 [details] [diff] [review]
wip
(Reporter)

Comment 5

5 years ago
follow the wip idea to replace ensureImageMapTree on new waitForImageMap.
Whiteboard: [good first bug][mentor=marco.zehe@googlemail.com][lang=js]
(Assignee)

Comment 6

5 years ago
Created attachment 633061 [details] [diff] [review]
Wip II

I followed surkovs example and modified all tests using the older ensureImageMapTree() function to use his new waitForImageMap() function.

All tests pass, but for the treeupdate/test_imagemap.html test, I had to leave / include the mouseover hack synthesizeMouse(), otherwise the test hangs and ... waits for a mouse over :)
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #633061 - Flags: feedback?(marco.zehe)

Comment 7

5 years ago
Comment on attachment 633061 [details] [diff] [review]
Wip II

Nice! f=me.
Attachment #633061 - Flags: feedback?(marco.zehe) → feedback+
(Assignee)

Comment 8

5 years ago
Comment on attachment 633061 [details] [diff] [review]
Wip II

Asking for review... does this satisfy the bug with that last mouseover hack still there?
Attachment #633061 - Flags: review?(dbolter)
Comment on attachment 633061 [details] [diff] [review]
Wip II

Review of attachment 633061 [details] [diff] [review]:
-----------------------------------------------------------------

Definitely progress, but test_imagemap is really the main thorn (bug 745788) so I'd like to get that figured out. Image map content/layout code is not the most fun.

If you like r=me but call this part 1 and leave the bug open please.
Attachment #633061 - Flags: review?(dbolter) → review+
Note I pondered (since pretest is so common) asking you to make that a util function, but since these are tests I actually personally like to see explicitly what's going on in each file anyways.
(Assignee)

Comment 11

5 years ago
Push to TRY:
https://tbpl.mozilla.org/?tree=Try&rev=d888ad9b1a5e

Orange on Linux indicating incorrect solution:

2622 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/bounds/test_zoom.html | Test timed out.
3502 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_docload.html | Different amount of expected children of [' no node info ', role: app root, name: 'Firefox', address: [xpconnect wrapped nsIAccessible]]. - got 3, expected 2
3509 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_docload.html | Different amount of expected children of [' no node info ', role: app root, name: 'Firefox', address: [xpconnect wrapped nsIAccessible]]. - got 3, expected 2
(Reporter)

Comment 12

5 years ago
You might want to keep mouse over thing for waitForImageMap function, this should keep working both treeupdate/test_imagemap.html and bounds/test_zoom.html
(Assignee)

Comment 13

5 years ago
Modified per Surkovs suggestion. re-try ...

https://tbpl.mozilla.org/?tree=Try&rev=8d168ec70845
(Assignee)

Comment 14

5 years ago
Created attachment 634008 [details] [diff] [review]
Patch (v3)

Updated to show what was pushed to TRY...
Attachment #615258 - Attachment is obsolete: true
Attachment #633061 - Attachment is obsolete: true
Attachment #634008 - Flags: review?(dbolter)
Comment on attachment 634008 [details] [diff] [review]
Patch (v3)

Review of attachment 634008 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. r=me if tests pass. (Bummer - I thought you'd run them on the earlier patch and somehow the image maps lazily instantiated on their own).
Attachment #634008 - Flags: review?(dbolter) → review+
(Assignee)

Comment 16

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=10e3d0212100
Whiteboard: [good first bug][mentor=marco.zehe@googlemail.com][lang=js] → [leave open][mentor=marco.zehe@googlemail.com][lang=js]
Target Milestone: --- → mozilla16
(Assignee)

Updated

5 years ago
Attachment #634008 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/10e3d0212100
(In reply to David Bolter [:davidb] from comment #9)
> Comment on attachment 633061 [details] [diff] [review]
> Wip II
> 
> Review of attachment 633061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Definitely progress, but test_imagemap is really the main thorn (bug 745788)
> so I'd like to get that figured out. Image map content/layout code is not
> the most fun.
> 
> If you like r=me but call this part 1 and leave the bug open please.

I don't understand my own comment.

Mark this should fix test_imagemap too right? (If you agree, close it fixed)
(Assignee)

Comment 19

5 years ago
It was my understanding that this change we just moved would close out this bug but not correct the issue with bug 745788. and indeed that bug had a fail last night at or around the time this patch went in. 

Note that several people have had a hand in the discussion here, and several bugs / issues are referred to over time:

   (bug 569425) can't get accessible
   (bug 741474) Different amount of expected children
   (Bug 570322) This one - random fail of image map (area) accessible creation
   (Bug 745788) Intermittent a11y/accessible/treeupdate/test_imagemap.html | Test timed out

We may have begun chasing our tail?

I'd suggest that we close this bug out, and focus fresh on what's wrong with the last remaining open bug 745788.
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.