1254232 - Narrate reads the period at the end of a paragraph, and spells out the next word

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect)

Description

[(no longer active)]

STR:

1. Go to <http://blog.monotonous.org/2016/03/07/narrate-a-new-feature-in-firefox-nightly/> and enter reader mode.
2. Click on Narrate and listen.

Narrate speaks the following at the end of the first paragraph:

... "reading pleasure dot A S of today"

Comment 1

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

I can't repro on my MBP.

Comment 2

[Eitan Isaacson [:eeejay]]

Looks like we need to tune the selector to break up that post into paragraphs. I'm surprised this comes up here since it is a standard wp post.

Comment 3

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

(In reply to Eitan Isaacson [:eeejay] from comment #2)
> Looks like we need to tune the selector to break up that post into
> paragraphs. I'm surprised this comes up here since it is a standard wp post.

I can repro the paragraphs not being broken up for most (all) articles I've tried today. Note this sounds like a different issue than comment 0.

Comment 4

[Eitan Isaacson [:eeejay]]

(In reply to David Bolter [:davidb] from comment #3)
> (In reply to Eitan Isaacson [:eeejay] from comment #2)
> > Looks like we need to tune the selector to break up that post into
> > paragraphs. I'm surprised this comes up here since it is a standard wp post.
> 
> I can repro the paragraphs not being broken up for most (all) articles I've
> tried today. Note this sounds like a different issue than comment 0.

I can reproduce what Ehsan is getting too. I think we are getting textContent of the container of all the paragraphs, and the text is mushed together. In the case over here, the period is followed by "As", the speech engine parses that as a hostname/url and says pleasure.as ("pleasure dot A S").

So that misspoken word is indicitive of a bigger problem.

What oher articles do you see this happening in?

Comment 5

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

(In reply to Eitan Isaacson [:eeejay] from comment #4)
> (In reply to David Bolter [:davidb] from comment #3)
> > (In reply to Eitan Isaacson [:eeejay] from comment #2)

> What oher articles do you see this happening in?

Paragraphs seem not chunked for:
e.g. http://www.cnn.com/2016/03/07/middleeast/middle-east-weapons-seizure/index.html
e.g. http://news.softpedia.com/news/facebook-fixes-bug-that-allowed-users-to-set-new-passwords-for-other-accounts-501449.shtml

(You have to get past title and author, then the rest is lumped together)

Comment 6

[(no longer active)]

I also get this behavior on http://www.cnn.com/2016/03/07/politics/nancy-reagan-funeral-friday/index.html.

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r=Gijs

Review commit: https://reviewboard.mozilla.org/r/38533/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38533/

Comment 8

[Eitan Isaacson [:eeejay]]

I originally thought some CSS selectors were enough to break up the paragraphs reliably. I was wrong. Above is a patch that uses a tree walker to get paragraphs.

Comment 9

[Eitan Isaacson [:eeejay]]

Comment on attachment 8727598 [details]
MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r=Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38533/diff/1-2/

Comment 10

[:Gijs (out for now; he/him)]

Comment on attachment 8727598 [details]
MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r=Gijs

https://reviewboard.mozilla.org/r/38533/#review35287

::: toolkit/components/narrate/Narrator.jsm:216
(Diff revision 2)
> +        // _speakInner advanced to the next node for us.

We don't call speakInner here, though... so this comment doesn't really make sense. Can you clarify?

::: toolkit/components/narrate/test/browser_narrate.js:54
(Diff revision 2)
> -    is(speechinfo.paragraph, 1, "second paragraph is being spoken");
> +    isnot(speechinfo.paragraph, paragraph, "next paragraph is being spoken");

Can we verify explicitly that speechino.paragraph corresponds to paragraph.nextElementSibling or something of that sort?

Comment 11

[Eitan Isaacson [:eeejay]]

https://reviewboard.mozilla.org/r/38533/#review35287

> Can we verify explicitly that speechino.paragraph corresponds to paragraph.nextElementSibling or something of that sort?

as discussed on IRC, this is tough!

Comment 12

[Eitan Isaacson [:eeejay]]

Comment on attachment 8727598 [details]
MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r=Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38533/diff/2-3/

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26d0246340d3

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/26d0246340d3

Comment 15

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Aurora 48.0a2 (buildID: 20160504004015).