507382 - focus is fired earlier than root accessible name is changed when switching between tabs

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: mochitest

focus is fired earlier than root accessible name is changed when switching between tabs of tabbrowser

Comment 1

[alexander :surkov (:asurkov)]

So DOM events are fired in correct order, select and then focus. When we get focus then @title attribute is updated already. But accessible name might be not updated yet. I guess problem is in nsIBaseWindow::GetTitle which is used in nsRootAccessible::GetName().

Comment 2

[alexander :surkov (:asurkov)]

Attachment: mochitest2

This mochitest doesn't demonstrate the bug if events debug dump is disabled. Probably timing issue.

Comment 3

[alexander :surkov (:asurkov)]

Ttile is changed on async event, see http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#4977. That's the problem.

Comment 4

[alexander :surkov (:asurkov)]

Note, getting name from title and title caching in nsXULWindow.cpp has been introduced in bug 257739.

Comment 5

[alexander :surkov (:asurkov)]

I wonder if nsDocument::GetTitle will work for us (keeping in mind SVG since it updates document title as well, see http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGTitleElement.cpp#192). 

If it is then should we remove the code of nsXULWindow.cpp introduced in bug 257739?

CC'ing Neil since he took a part in discussion of the bug 257739.

Comment 6

[[e-mail]]

(In reply to comment #5)
> should we remove the code of nsXULWindow.cpp introduced in bug 257739?
Since that code implemented previously unimplemented interface methods I don't quite see the point of unimplementing them again...

Comment 7

[alexander :surkov (:asurkov)]

changing blocking since there is nothing wrong with a11y events here.

Comment 8

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 9

[[e-mail]]

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

I doubt that your code change is big enough to merit superreview ;-)

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> (From update of attachment 391778 [details] [diff] [review])
> I doubt that your code change is big enough to merit superreview ;-)

Ah, just to ensure nsIDOMNSDocument::GetTitle is all we look for :)

Comment 11

[Marco Zehe (:MarcoZ)]

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

>+    <a target="_blank"
>+       href="https://bugzilla.mozilla.org/show_bug.cgi?id="
>+       title="">
>+      Mozilla Bug 
>+    </a>

This looks incomplete to me.

Also, what's the chmod change for relations.js in this patch?

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

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

r=me thanks!

Comment 13

[alexander :surkov (:asurkov)]

checked in with Marco's comments addressed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/adb5151a10eb

Comment 14

[alexander :surkov (:asurkov)]

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

it's urgent for AT users to hear correct page name when they are switched between tabs. Simple fix.

Comment 15

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090822 Minefield/3.7a1pre (.NET CLR 3.5.30729)

Comment 16

[alexander :surkov (:asurkov)]

landed on 1.9.2 - http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8f632ea6e803

Comment 17

[alexander :surkov (:asurkov)]

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

originally bug was reported under 3.5 firefox version, important for AT users, simple fix.

Comment 18

[Daniel Veditz [:dveditz]]

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

Approved for 1.9.1.4, a=dveditz -- but must be landed by Tuesday (sept 22).

Comment 19

[Marco Zehe (:MarcoZ)]

Alex, can you take care of landing this? This is using portions of our events testing which we haven't backported to 1.9.1 yet.

Comment 20

[Marco Zehe (:MarcoZ)]

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

Oops, forgot that Alex is attending a conference this week and won't be able to complete this in time. Deferring it to 1.9.1.5 since the 1.9.1 version of this patch needs some delicate porting.

Comment 21

[Marco Zehe (:MarcoZ)]

Verified fixed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090922 Namoroka/3.6b1pre (.NET CLR 3.5.30729)

Comment 22

[Daniel Veditz [:dveditz]]

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

"delicate porting" doesn't instill us will confidence, but in any case indicates this is definitely not the patch you mean to request approval on.

Comment 23

[alexander :surkov (:asurkov)]

Daniel, sorry, I didn't get your point. Why do you think this patch shouldn't be landed on 1.9.1?

Comment 24

[Daniel Veditz [:dveditz]]

comment 20 says we need a different patch for 1.9.1.

I interpreted "delicate" as "tricky", but maybe it means "minor"? Either way, get us a patch that applies to 1.9.1 (that you've tested builds on 1.9.1 and fixes the bug there) before requesting approval on it.

Since this is not
  1) a security fix
  2) a top crash fix, or
  3) fixing a regression from an earlier stable-branch patch
the presumption is that we're not taking it for a stable branch release.

https://wiki.mozilla.org/TreeStatus#mozilla-1.9.1_-_1.9.1_Branch_.28Firefox_3.5.x.2C_Gecko_1.9.1_work.29

Comment 25

[alexander :surkov (:asurkov)]

If I'll update patch to 1.9.1 and test it then can I ask approval again even if this patch is not security fix and etc? Because this patch makes AT user life easier when they switch between tabs, otherwise they won't know which tab they chose.