Narrate stops when it gets to a figure inside the article

VERIFIED FIXED in Firefox 48

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Ehsan, Unassigned)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 verified)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
STR: Narrate this page: <http://haakonsk.blogg.no/1456259429_if_youre_alive_in_30_.html>

The narration stops at "remaining life expectancy can be illustrated like this" right before we get to the first figure in the article.
Reporter

Comment 1

3 years ago
Seems like pressing stop will make the narration continue...
The other problem here is that we are getting that error in osx because we are sending an empty string to synthesis. I think that is not good. If anything, because of fingerprinting.
Comment on attachment 8728053 [details]
MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r=Gijs

https://reviewboard.mozilla.org/r/38769/#review35455

I'm a little uncomfortable with the assumption here that for every error we should just try reading the next paragraph. Under what circumstances should we throw the error event per spec? Could it be temporary, in which case, shouldn't we be restarting where we left off?

It seems to me like we could/should use the treewalker to filter out the images and potentially other empty bits of paragraph or whatever?

If we do have to (partially?) rely on the error handler here, I wonder if it needs some kind of backoff/limit. If narration is somehow busted because platform support is missing or causing errors (say, because you stored a voice in the prefs and that voice is now no longer available, or because whatever espeak-type API we use on linux is not installed, or just because software is sometimes awful) then right now it will try to speak every paragraph in turn until it hits the end, all the while not emitting noise (well, or a series of beeps - we don't really know in the general case), and ending up with the page scrolled to the bottom and the user wondering what on earth just happened. That seems unfortunate as well. :-)
Attachment #8728053 - Flags: review?(gijskruitbosch+bugs)
https://reviewboard.mozilla.org/r/38769/#review35455

Yup, all legitimate concerns.. I'll revise the patch to reject the promise on error. And for this specific case, I'll have the tree filter remove empty nodes.
Attachment #8728053 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8728053 [details]
MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r=Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38769/diff/1-2/
Comment on attachment 8728053 [details]
MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r=Gijs

https://reviewboard.mozilla.org/r/38769/#review35807
Attachment #8728053 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8728591 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8728591 [details]
MozReview Request: Bug 1254526 - Filter out empty text nodes from narrate queue. r=Gijs

https://reviewboard.mozilla.org/r/38983/#review35809

Nice, thanks for the extra comment!

::: toolkit/components/narrate/Narrator.jsm:69
(Diff revision 1)
> +          if (!node.textContent.match(/\S/)) {

Nit:
```
!/\S/.test(node.textContent)
```

is slightly cheaper than `.match`
Comment on attachment 8728053 [details]
MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r=Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38769/diff/2-3/
Attachment #8728053 - Attachment description: MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r?Gijs → MozReview Request: Bug 1254526 - Don't let Narrate get into bad state after encountering a synth error. r=Gijs
Comment on attachment 8728591 [details]
MozReview Request: Bug 1254526 - Filter out empty text nodes from narrate queue. r=Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38983/diff/1-2/
Attachment #8728591 - Attachment description: MozReview Request: Bug 1254526 - Filter out empty text nodes from narrate queue. r?Gijs → MozReview Request: Bug 1254526 - Filter out empty text nodes from narrate queue. r=Gijs

Comment 13

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa1daaba023b
https://hg.mozilla.org/mozilla-central/rev/5b6bcc1fec08
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Aurora 48.0a2 (buildID: 20160504004015).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.