Bug 26995 - AX: Some webpages do not send AXLoadComplete
Summary: AX: Some webpages do not send AXLoadComplete
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: chris fleizach
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-06 11:50 PDT by chris fleizach
Modified: 2009-07-07 14:06 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (623 bytes, patch)
2009-07-06 16:08 PDT, Greg Hughes
darin: review-
Details | Formatted Diff | Diff
Updated patch (1.56 KB, application/octet-stream)
2009-07-06 18:50 PDT, Greg Hughes
no flags Details
Updated patch 3 (2.58 KB, patch)
2009-07-06 19:17 PDT, Greg Hughes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2009-07-06 11:50:21 PDT
some webpages are not sending AXLoadComplete notifications
Comment 1 Greg Hughes 2009-07-06 16:08:28 PDT
Created attachment 32332 [details]
Proposed patch

Patch ensures that the AXCache is updated with the current document before posting the AXLoadComplete notification.
Comment 2 chris fleizach 2009-07-06 16:10:46 PDT
you could probably cache renderer()
you might want to add a comment saying why we need to do this
Comment 3 Darin Adler 2009-07-06 18:00:42 PDT
Comment on attachment 32332 [details]
Proposed patch

Code change looks fine. Needs comment explaining why it's calling getOrCreate and discarding the result. Needs a ChangeLog.

review- because of those issues
Comment 4 chris fleizach 2009-07-06 18:01:50 PDT
greg, also run the prepare-ChangeLog script and re-submit the patch (along with comments), which will obviate this patch
Comment 5 Greg Hughes 2009-07-06 18:50:42 PDT
Created attachment 32350 [details]
Updated patch

patch updated to cache renderer() and include a comment
Comment 6 Greg Hughes 2009-07-06 19:17:04 PDT
Created attachment 32351 [details]
Updated patch 3

Updated patch based on review
Comment 7 Darin Adler 2009-07-07 00:54:15 PDT
Comment on attachment 32351 [details]
Updated patch 3

> +    RenderObject* render = renderer();

An idiom we use for this is

    RenderObject* renderer = this->renderer();

The downside of naming the variable "render" is that's a verb with a different meaning.

Is there any way to make a regression test for this?

r=me
Comment 8 chris fleizach 2009-07-07 08:35:22 PDT
for a regression test... what if we stored the last notification to be sent in AXObjectCache. then in the layout test we could have a mechanism to ask for the last notification, which would allow us to ensure it was sent. 

it seems like we might want to also add an ASSERT checking that there is an ax object available before posting the notification, so that we might catch other cases where this is happening
Comment 9 Darin Adler 2009-07-07 10:10:35 PDT
(In reply to comment #8)
> it seems like we might want to also add an ASSERT checking that there is an ax
> object available before posting the notification, so that we might catch other
> cases where this is happening

We can't do that until we fix the real problem, because we know that assert will fire.
Comment 10 chris fleizach 2009-07-07 11:54:01 PDT
i was not able to write a layout test for this because i need the layout test to stall until it the notification gets posted. unfortunately, the layout tests get run and exited before the notification is actually sent.
Comment 11 chris fleizach 2009-07-07 12:06:04 PDT
http://trac.webkit.org/changeset/45602
Comment 12 Darin Adler 2009-07-07 14:06:57 PDT
(In reply to comment #10)
> i was not able to write a layout test for this because i need the layout test
> to stall until it the notification gets posted. unfortunately, the layout tests
> get run and exited before the notification is actually sent.

That's something we can tackle later; it can be overcome so we can make a test for this. You can call layoutTestController.waitUntilDone() to prevent the layout test machinery from moving on to the next test until layoutTestController.notifyDone() is called. Then you just have to figure out how to get some JavaScript code called once the test truly is done.