906666 - focus may be lost when navigating back the visited pages

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

were reported by AT vendor:

1) first go to:
http://www.freedomscientific.com/training/Surfs-Up/_Surfs_Up_Start_Here.htm
2) then select the link that says:
"Navigating Web Pages"
3) After the new page loads, then open www.freedomscientific.com by typing in the address bar.
4) press alt+left arrow to move back to the previous page.
I see a document loaded event, followed by a state changed event and finally a focus changed event.
5) press alt + left arrow again to move back to my surf's up page.  This time I see a document loaded event followed by the state changed event, but no focus changed event.

(nomination to 2013q3a11y since lost focus is a huge problem for AT users)

Comment 1

[alexander :surkov (:asurkov)]

Max, Yuriy, you might want to consider this bug if you want to get some challenge debugging - focus problems are always tricky. And yes it's very important for our users.

Comment 2

[James Teh [:Jamie]]

NVDA users have been reporting this for a while, but i haven't been able to reproduce it reliably:
http://community.nvda-project.org/ticket/3051
It's definitely very annoying for NVDA + Firefox users.

Comment 3

[alexander :surkov (:asurkov)]

so the problem here like in bug 660083 we get DOM focus event "too early":

A11Y FOCUS: DOM focus; 51:00.279
  {
    [not accessible] Target: 0A564C50, document
    [contained by not accessible document]:
    DOM document: 0A564C50, acc document: 00000000
    uri: http://www.freedomscientific.com/training/Surfs-Up/_Surfs_Up_Start_Here.htm
    docshell busy: 'busy', 'before page load'; content document
    docshell hierarchy, parent: 04E821A0, root: 04E821A0, is tab document: yes;
    doc state: complete, not initial, not showing, not visible, not visible considering ancestors, active, not resource, has role content
    presshell: 09F20DA8, is not destroying, root scroll frame: 09FDC320
    load group: 08180CC0, parent id: 04F5AA98
    parent uri: chrome://browser/content/browser.xul


  }

the document is *not* visible and thus we reject to create an accessible for it (IsVisibleConsideringAncestors check). I would love to hear your thoughts how to bypass it.

Comment 4

[Trevor Saunders (:tbsaunde)]

> the document is *not* visible and thus we reject to create an accessible for
> it (IsVisibleConsideringAncestors check). I would love to hear your thoughts
> how to bypass it.

I wonder if it really makes sense for it to be getting focus then...

Comment 5

[alexander :surkov (:asurkov)]

after all it's just a DOM event, maybe not really connected to visual part.

Cc'ing Enn to comment it

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #5)
> after all it's just a DOM event, maybe not really connected to visual part.

Well, sure but its a dom event that tells you what has focus and it doesn't really seem to make sense that focused thing isn't visible

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #6)
> (In reply to alexander :surkov from comment #5)
> > after all it's just a DOM event, maybe not really connected to visual part.
> 
> Well, sure but its a dom event that tells you what has focus and it doesn't
> really seem to make sense that focused thing isn't visible

content and layout are async things in general

Comment 8

[alexander :surkov (:asurkov)]

If we solved it on our side then how would it look? Would it work if we created a document early but kept it as hanging document until all our requirements are fulfill or until a timer runs out?

Trev, ideas?

Comment 9

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #8)
> If we solved it on our side then how would it look? Would it work if we
> created a document early but kept it as hanging document until all our
> requirements are fulfill or until a timer runs out?

I'd really rather not do that

Comment 10

[alexander :surkov (:asurkov)]

alternatives?

Comment 11

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #10)
> alternatives?

find out from dom / focus people why its sane for non visible document to have focus

Comment 12

[Neil Deakin]

You shouldn't be receiving a focus event if IsVisibleConsideringAncestors returns false. I couldn't reproduce this myself, but if you have a failing case with NSPR_LOG_MODULES=Focus:5 set, please post it here.

Comment 13

[alexander :surkov (:asurkov)]

Attachment: DOM focus log

Comment 14

[alexander :surkov (:asurkov)]

Attachment: a11y focus log

Comment 15

[alexander :surkov (:asurkov)]

(In reply to Neil Deakin from comment #12)
> You shouldn't be receiving a focus event if IsVisibleConsideringAncestors
> returns false. I couldn't reproduce this myself, but if you have a failing
> case with NSPR_LOG_MODULES=Focus:5 set, please post it here.

maybe a11y status matters, try to repeat steps with inspector.exe running for example (in case if attached log is not helpful). btw, thank you for looking at the issue

Comment 16

[Neil Deakin]

The log suggests that nsGlobalWindow::RestoreWindowState is being called during step 5 after the Navigating.htm page is already hidden but before the new page _Surfs_Up_Start_Here.htm is displayed. nsGlobalWindow::RestoreWindowState should be called after step 4 as the focus call inside this method is used to restore focus to the link that was clicked.

Are you waiting for the pages to fully load before navigating? Is there a way you could indicate in the focus log which part of the log applies to which step?

It would also be useful to know if this happens without accessibility.

Comment 17

[alexander :surkov (:asurkov)]

(In reply to Neil Deakin from comment #16)
> The log suggests that nsGlobalWindow::RestoreWindowState is being called
> during step 5 after the Navigating.htm page is already hidden but before the
> new page _Surfs_Up_Start_Here.htm is displayed.
> nsGlobalWindow::RestoreWindowState should be called after step 4 as the
> focus call inside this method is used to restore focus to the link that was
> clicked.

so is it a bug?

> Are you waiting for the pages to fully load before navigating? 

I think so, I waited about 3 sec between each step

> Is there a
> way you could indicate in the focus log which part of the log applies to
> which step?

I don't know NSPR logging. If it can be redirected to output then it can be mixed with a11y logging. Or alternatively if there's other NSPR logging for documents loading.

> It would also be useful to know if this happens without accessibility.

you said you can't reproduce it so a11y plays some role here I assume. I could repeat steps on my machine without a11y running and attach the log. Should I?

Comment 18

[Neil Deakin]

(In reply to alexander :surkov from comment #17)
> so is it a bug?

Yes, but I don't know where.

> I don't know NSPR logging. If it can be redirected to output then it can be
> mixed with a11y logging. Or alternatively if there's other NSPR logging for
> documents loading.

nspr logging goes to stderr by default.

> you said you can't reproduce it so a11y plays some role here I assume. I
> could repeat steps on my machine without a11y running and attach the log.
> Should I?

sure.

Comment 19

[alexander :surkov (:asurkov)]

Attachment: (no a11y enabled) NSPR focus log

Comment 20

[alexander :surkov (:asurkov)]

Attachment: a11y enabled (NSPR focus + a11y focus log)

a11y logs DOM focus and DOM blur, also prints time when events were caught, and document uri the event target is in, I hope it will be helpful for easier understanding the NSPR logging

Comment 21

[Neil Deakin]

(In reply to alexander :surkov from comment #19)
> Created attachment 810693 [details]
> (no a11y enabled) NSPR focus log

That log shows the same issue, so this is a bug somewhere else.

Comment 22

[alexander :surkov (:asurkov)]

do you have ideas where to start or maybe recommend somebody to look?

Comment 23

[Neil Deakin]

Attachment: testing patch

I managed to reproduce this, but can't test the accessibility parts. Can you try this patch?

Comment 24

[alexander :surkov (:asurkov)]

(In reply to Neil Deakin from comment #23)
> Created attachment 810719 [details] [diff] [review]
> testing patch
> 
> I managed to reproduce this, but can't test the accessibility parts. Can you
> try this patch?

yes it works

Comment 25

[Neil Deakin]

Attachment: Alternate patch

This patch is better, but I need to think more about the implications of this change.

Comment 26

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

(In reply to Neil Deakin from comment #25)
> Created attachment 812922 [details] [diff] [review]
> Alternate patch
> 
> This patch is better, but I need to think more about the implications of
> this change.

Neil, how goes the thinking? :)

Comment 27

[alexander :surkov (:asurkov)]

Neil, friendly ping. This one is very important for accessibility since it must keep the users quite annoying.

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 812922 [details] [diff] [review]
Alternate patch

Could be if (!innerWindow || innerWindow->IsFrozen())
since we sure shouldn't think a window without innerwindow is really visible.

Comment 29

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85977fa93cb4

Comment 30

[Neil Deakin]

Alexander, do you have a way to write a test for this?

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/85977fa93cb4

Comment 32

[alexander :surkov (:asurkov)]

(In reply to Neil Deakin from comment #30)
> Alexander, do you have a way to write a test for this?

Unfortunately no since I don't have reliable steps to reproduce (str from bug description aren't suitable for our test suite since it relies on real web site).