175842 - Incorrect window handle for events related to a new document being ready

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

When a new document starts to load, we fire an EVENT_STATE_CHANGE on the
ROLE_PANE object (which is the nsRootAccessible for the content document). We
fire this with the window handle associated with the window about to be
destroyed. At this point, ROLE_PANE object has the STATE_BUSY flag set.

We also fire an EVENT_STATE_CHANGE with STATE_BUSY cleared when the new document
is ready.

However, we are firing that event with the old window handle, when we should be
firing it with the window handle of the newly created window.

Comment 1

[Aaron Leventhal]

Attachment: Make each RootAccessible for a content doc have it's own webprogresslistener and fire EVENT_STATE_CHANGE on a newly formed RootAccessible when it's finished loading

When a new document is loading, the nsRootAccessible for the old document is
still around while the nsRootAccessible for the new document is constructed.

We use a variable called mIsNewDocument, to keep track of whether this
nsRootAccessible is for the new or old document.

The EVENT_STATE_CHANGE with STATE_BUSY is only fired for the old document, when
the location changes -- which indicates a new load is about to commence.

The EVENT_STATE_CHANGE with STATE_BUSY cleared is only fired for the new
document when the content is finished loading.

Comment 2

[Kyle Yuan]

Comment on attachment 103799 [details] [diff] [review]
Make each RootAccessible for a content doc have it's own webprogresslistener and fire EVENT_STATE_CHANGE on a newly formed RootAccessible when it's finished loading

r=kyle

For the ATK part, we will decide whether we need 2 events or not later, and
file a new bug if needed.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 103799 [details] [diff] [review]
Make each RootAccessible for a content doc have it's own webprogresslistener and fire EVENT_STATE_CHANGE on a newly formed RootAccessible when it's finished loading

- In nsRootAccessible::AddContentDocListeners():

+  nsCOMPtr<nsIScriptGlobalObject> scriptGlobal;
+  mDocument->GetScriptGlobalObject(getter_AddRefs(scriptGlobal));
+  if (!scriptGlobal)
+    return;
+
+  nsCOMPtr<nsIDocShell> docShell;
+  scriptGlobal->GetDocShell(getter_AddRefs(docShell));
+  nsCOMPtr<nsIDocShellTreeItem> docShellTreeItem(do_QueryInterface(docShell));

You could do the above with this code in stead:

+  nsCOMPtr<nsISupports> container;
+  mDocument->GetContainer(getter_AddRefs(container));
+
+  nsCOMPtr<nsIDocShellTreeItem>
docShellTreeItem(do_QueryInterface(container));

Less code, and fewer dependencies.

...
+      mDocLoadTimer->InitWithFuncCallback(DocLoadCallback, this, 1,
+				     nsITimer::TYPE_ONE_SHOT);

Indentation of second line arguments not right...

Other than that, sr=jst

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 103799 [details] [diff] [review]
Make each RootAccessible for a content doc have it's own webprogresslistener and fire EVENT_STATE_CHANGE on a newly formed RootAccessible when it's finished loading

a=roc+moz for trunk

Comment 5

[Aaron Leventhal]

checked in