1280551 - Documents sometimes load with no child accessibles but all text merged into document

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Sometimes, documents load with no child accessibles except text leaf accessibles.
* All the text is merged into the parent; i.e. retrieving the text on the document returns all of the text with no embedded object characters.
* All of the text content is present as you would expect; there is just no semantic information whatsoever.
* If you tab to something which should be focusable, focus does move in Firefox (I can confirm this by using copy link location in the context menu or similar), but no accessible event is fired.
* The only way to fix this is to refresh the document, and even then, it might (or might not) occur again.

Unfortunately, this is rather intermittent. I currently can't get it to happen in Firefox, though I've definitely seen it happen there. It happens far more frequently in Thunderbird. It's a fairly recent regression; I'd say some time in the last month or so.

Sorry for filing a bug with such little info. I really don't have any more info to give, but I figured filing it with little info was better than never filing it. :(

Comment 1

[Marco Zehe (:MarcoZ)]

I can reproduce it with git manpages. So in Windows, if I have Git installed from the official distribution, open a Git bash, then type

$ git help grep

Firefox comes up with the man page for the grep documentation.

If I do this a couple of times, eventually the man page will come up as a big blob of text without headings or other accessibles. It may require 3, 4, or 5 attempts.

Comment 2

[alexander :surkov (:asurkov)]

I'd be helpful to know a regression bug.

Comment 3

[James Teh [:Jamie]]

I just (painstakingly :)) tracked this down with mozregression. The regressing bug is bug 1261170.

26:59.64 INFO: Last good revision: 336759fb7df024ec7fe456df8337316e8361c4d5
26:59.64 INFO: First bad revision: 296675bfe330a157e8b3054befee29273e3ac3fb
26:59.64 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=336759fb7df024ec7fe456df8337316e8361c4d5&tochange=296675bfe330a157e8b3054befee29273e3ac3fb

27:00.87 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1261170

Comment 4

[Marco Zehe (:MarcoZ)]

Wow, thanks, Jamie, for sticking with it and finding the regressing bug!

Alex, any ideas?

Comment 5

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #3)
> I just (painstakingly :)) tracked this down with mozregression. The
> regressing bug is bug 1261170.

thank you, Jamie. 

The finding is interesting because bug 1292627 largely reverted that change, and text node insertions and element node insertions have common processing path again.

what are known reliable steps to reproduce?

Marco, what git version do you use or what package did you use to install it? any change to see that on mac?

Comment 6

[Marco Zehe (:MarcoZ)]

(In reply to alexander :surkov from comment #5)
> what are known reliable steps to reproduce?

I managed to do it with calling git help commit or such a command, which on Windows, brings up the default browser (or the currently running instance of Firefox that is running), with the HTML manual page. That is the standard Git for Windows install from https://git-scm.com/. I usually had to bring up the same man page two or three times before the bug became visible.

> Marco, what git version do you use or what package did you use to install
> it? any change to see that on mac?

I never saw Git for Mac bring up the man page in a browser like that. It always outputs to the console, and I've never bothered to check whether that can be changed.Unix command line and I... ;)

Comment 7

[alexander :surkov (:asurkov)]

I tried 8 times git help grep and NVDA managed to read all headings every time. Marco, may I ask you to send logs with export A11YLOG=tree,verbose; variable set?

Comment 8

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

Marco are you able to get logs as per comment 7?

Comment 9

[Marco Zehe (:MarcoZ)]

No luck so far. :-(

Comment 10

[James Teh [:Jamie]]

Attachment: Verbose a11y tree log.

I managed to get this to happen on the first try in this session.

Comment 11

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #10)
> Created attachment 8784708 [details]
> Verbose a11y tree log.
> 
> I managed to get this to happen on the first try in this session.

thank you, Jamie. what a page that was?

Comment 12

[alexander :surkov (:asurkov)]

btw, it would be useful to have a normal log too to compare

Comment 13

[James Teh [:Jamie]]

Attachment: Help page for "git log", which I launched from the shell to reproduce the problem.

Comment 14

[James Teh [:Jamie]]

Attachment: Verbose a11y tree log for a successful render. Same STR, just different result.

Comment 15

[alexander :surkov (:asurkov)]

Jamie, are you sure 'verbose' is set too? All verbose logs are missed. I'd be good to have one for 'bad case', but otherwise it looks like layout issue. In a bad case we get tons of text change notifications, which are under document accessible, which means 'initial tree creation' failed to create a tree. Also no other notifications from layout. It looks like frames are not accessible when we build the initial tree, so we skip them.

Comment 16

[James Teh [:Jamie]]

(In reply to alexander :surkov from comment #15)
> Jamie, are you sure 'verbose' is set too? All verbose logs are missed.

Hmm. I have the A11YLOG environment variable set to "tree,verbose;". Is that correct? Were both logs missing verbose info or just one of them?

Comment 17

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #16)
> (In reply to alexander :surkov from comment #15)
> > Jamie, are you sure 'verbose' is set too? All verbose logs are missed.
> 
> Hmm. I have the A11YLOG environment variable set to "tree,verbose;". 

it's strange, any chance for misspelling?

> Is that
> correct? 

yes

> Were both logs missing verbose info or just one of them?

both of them

here's a try build that contains some noisy extra logging ("tree") that may reveal a problem, would you please give me a log from that build too?

Comment 18

[James Teh [:Jamie]]

> > Hmm. I have the A11YLOG environment variable set to "tree,verbose;". 
> 
> it's strange, any chance for misspelling?
I copied and pasted in both cases, so I don't think so. Does verbose logging work for normal nightly builds? I'm just using a nightly, not a local build.

> here's a try build that contains some noisy extra logging ("tree") that may
> reveal a problem, would you please give me a log from that build too?
I don't see a link to a try build...?

Comment 19

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #18)
> > > Hmm. I have the A11YLOG environment variable set to "tree,verbose;". 
> > 
> > it's strange, any chance for misspelling?
> I copied and pasted in both cases, so I don't think so.

hm, "export A11YLOG=tree,verbose;" should do a trick. Once verbose logging is enabled, then the log should contain messages like "Initial subtree;" and "children after insertion"

> Does verbose logging
> work for normal nightly builds? I'm just using a nightly, not a local build.

it should be no difference, same module, just different flags

> > here's a try build that contains some noisy extra logging ("tree") that may
> > reveal a problem, would you please give me a log from that build too?
> I don't see a link to a try build...?

forgot to paste it, sorry, https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-95ad8317c194efcc72c3744bbb9253e5a28029f1/

Comment 20

[James Teh [:Jamie]]

In Windows, it's "set", not "export", but the effect is the same.

I just had a thought: is verbose logging perhaps output to stderr, not stdout? I was only redirecting stdout (and because it's a GUI mode app, stdout/tstderr go nowhere if not redirected).

Comment 21

[James Teh [:Jamie]]

Attachment: Tree verbose a11y log from try build when document breaks.

Comment 22

[James Teh [:Jamie]]

Attachment: Tree verbose a11y log from try build when document works as expected.

Comment 23

[alexander :surkov (:asurkov)]

jamie, thanks for the logs, I started a new build with forced tree and verbose logging [1], would you please share a bad log when the build is complete?

[1]  https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-080f67a1259365ea553f759829e7d8fe68f14fd2/

Comment 24

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #20)
> In Windows, it's "set", not "export", but the effect is the same.

I think I do 'export' on Windows (in mozbuild console I guess), I might be wrong but I thought that the difference is 'export' lives during the session, while 'set' is active in scope of a command.

> I just had a thought: is verbose logging perhaps output to stderr, not
> stdout? I was only redirecting stdout (and because it's a GUI mode app,
> stdout/tstderr go nowhere if not redirected).

they are quite same, they are just different modules. When build is started and logging is enabled, then it outputs what modules was enabled, in your logs it always says 'module enabled: tree' and that's it. Not sure why.

Comment 25

[James Teh [:Jamie]]

Attachment: Verbose a11y tree log for failed render.

Comment 26

[James Teh [:Jamie]]

Attachment: Verbose a11y tree log for successful render.

Comment 27

[James Teh [:Jamie]]

(In reply to alexander :surkov from comment #24)
> > In Windows, it's "set", not "export", but the effect is the same.
> I think I do 'export' on Windows (in mozbuild console I guess)

Oh! I'm not using the mozbuild console, just the normal Windows command interpreter, which is very different to bash.

This made me realise that I assumed you wanted a literal semi-colon at the end; i.e. so Firefox would see "tree,verbose;". Actually, the semi-colon would have been interpreted as a command separator in bash, so for "export A11YLOG=tree,verbose;", Firefox would never see the semi-colon. In contrast, semi-colon isn't special for the Windows command interpreter, so Firefox would see it. That's probably why this wasn't working; it would have been ignoring the invalid token "verbose;". Anyway, I generated these latest logs with the try build you provided just to be sure.

It's worth noting that this bug has made it into Firefox 48 and it's hurting more users than me now. I'm also seeing it a lot lately on GitHub and in other places. It'd be great if this could be given some priority for a fix in Firefox 49. I'm having to switch browsers from time to time as a result of this.

Comment 28

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #27)
> (In reply to alexander :surkov from comment #24)
> > > In Windows, it's "set", not "export", but the effect is the same.
> > I think I do 'export' on Windows (in mozbuild console I guess)
> 
> Oh! I'm not using the mozbuild console, just the normal Windows command
> interpreter, which is very different to bash.
> 
> This made me realise that I assumed you wanted a literal semi-colon at the
> end; i.e. so Firefox would see "tree,verbose;". Actually, the semi-colon
> would have been interpreted as a command separator in bash, so for "export
> A11YLOG=tree,verbose;", Firefox would never see the semi-colon. In contrast,
> semi-colon isn't special for the Windows command interpreter, so Firefox
> would see it. That's probably why this wasn't working; it would have been
> ignoring the invalid token "verbose;". 

sounds true

> Anyway, I generated these latest logs
> with the try build you provided just to be sure.

thank you, again

> It's worth noting that this bug has made it into Firefox 48 and it's hurting
> more users than me now. I'm also seeing it a lot lately on GitHub and in
> other places. It'd be great if this could be given some priority for a fix
> in Firefox 49. I'm having to switch browsers from time to time as a result
> of this.

I wish I was able to debug it myself.

Due to some reason, we don't find any accessibles under a document, I'll start over a new build with even more extended logging.

https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-da3340dbff11701aa6c12d7f99a168da552d9def/

Comment 29

[James Teh [:Jamie]]

Attachment: Verbose a11y tree logs for failed and successful renders.

I had to zip this, as the fail log was too big to attach uncompressed. :(

Comment 30

[alexander :surkov (:asurkov)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=78927f3618ac

Comment 31

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #29)
> Created attachment 8792763 [details]
> Verbose a11y tree logs for failed and successful renders.
> 
> I had to zip this, as the fail log was too big to attach uncompressed. :(

thanks, logging files had names were confused, bad was good, good was bad :)

Would you please give a try to this build [1], no logging, it should fix the issue.

[1] https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-78927f3618acf9b02bb689e016cf4099d9ded169/

Comment 32

[James Teh [:Jamie]]

(In reply to alexander :surkov from comment #31)
> thanks, logging files had names were confused, bad was good, good was bad :)

Oops. Sorry. I originally named them badlog.txt and goodlog.txt and then renamed them to make more sense, but clearly, I renamed the wrong way. :(

> Would you please give a try to this build [1], no logging, it should fix the
> issue.

And it does! Thank you! We got there! :)

Ironically, now that you've solved this, we've just been presented with a test case that reproduces this issue 100%, at least for me. (Your try build does fix this.) In case it's helpful, STR:
1. Open this URL: http://codepen.io/PetrashkoEV/pen/BLzRjE?editors=1010
2. Press the "Open popup button" button.
3. Press the "Close popup button".
4. Examine the accessibility tree for the test document.
Expected: The first child should be a form.
Actual: There are only text leaf children.

Comment 33

[alexander :surkov (:asurkov)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b0c1a11dd3d

Comment 34

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #32)
> (In reply to alexander :surkov from comment #31)
> > thanks, logging files had names were confused, bad was good, good was bad :)
> 
> Oops. Sorry. I originally named them badlog.txt and goodlog.txt and then
> renamed them to make more sense, but clearly, I renamed the wrong way. :(

no worries, it took almost no time to find the right one 

> > Would you please give a try to this build [1], no logging, it should fix the
> > issue.
> 
> And it does! Thank you! We got there! :)

awesome!

> Ironically, now that you've solved this, we've just been presented with a
> test case that reproduces this issue 100%, at least for me. (Your try build
> does fix this.) In case it's helpful, STR:

thanks, I've got one in mochitest (btw, our mochitests were very close to cover the case, but still not there)

Comment 35

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 36

[Yura Zenevich [:yzen] (please use "needinfo")]

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

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

Fix seems trivial, thanks for the test

Comment 37

[alexander :surkov (:asurkov)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=53caa2d98fe0

Comment 38

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff755d190b023901788a288ae2b1c404eb9ef7d3
Bug 1280551 - Documents sometimes load with no child accessibles but all text merged into document, r=yzen

Comment 39

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ff755d190b02

Comment 40

[Marco Zehe (:MarcoZ)]

Alex, please request approval for aurora (51) and Beta (50). This problem is wide-spread, and it also fixes bug 1281828. David, do you know if there's a 49.0.1 or so planned which could take this as a ride-along as well?

Comment 41

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

I'm not aware of a planned dot release, but Liz is tracking all possible ride-alongs in case there is.

Comment 42

[alexander :surkov (:asurkov)]

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

Approval Request Comment
[Feature/regressing bug #]: 1261170
[User impact if declined]: web pages and browser UI intermittently inaccessilbe for screen reader users
[Describe test coverage new/current, TreeHerder]:mochitest, manual testing
[Risks and why]: low, a small change
[String/UUID change made/needed]:no

Comment 43

[Gerry Chang [:gchang]]

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

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

This patch fixes a regression. Take it in 51 aurora.

Comment 44

[Marco Zehe (:MarcoZ)]

Alex, can you also prepare a patch that can land on 50 (Beta) and possibly 49 dot release? The current patch is based on bug 1274381 and thus cannot be backported, but it fixes a regression from an older bug, so we need a patch based on the code predating bug 1274381. Thanks!

Comment 45

[alexander :surkov (:asurkov)]

Attachment: beta patch

here's a beta patch, hopefully it works ;)

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ee1b7429ed13

Comment 47

[Marco Zehe (:MarcoZ)]

Comment on attachment 8794876 [details] [diff] [review]
beta patch

Yes, this is correct.

Comment 48

[Marco Zehe (:MarcoZ)]

Comment on attachment 8794876 [details] [diff] [review]
beta patch

Approval Request Comment
[Feature/regressing bug #]: 1261170
[User impact if declined]: web pages and browser UI intermittently inaccessible for screen reader users, also impacts Thunderbird users
[Describe test coverage new/current, TreeHerder]:mochitest, manual testing, lives on Central and Aurora
[Risks and why]: low, a small change
[String/UUID change made/needed]:no

Comment 49

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8794876 [details] [diff] [review]
beta patch

Fixes an issue in Reader Mode, stabilized on pre-beta channels, Beta50+

Comment 50

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6354213ac26b