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

VERIFIED FIXED in Firefox 48

Status

()

Toolkit
Reader Mode
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: eeejay)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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"
I can't repro on my MBP.
(Assignee)

Comment 2

2 years ago
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.
(Assignee)

Updated

2 years ago
Assignee: nobody → eitan
(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.
(Assignee)

Comment 4

2 years ago
(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?
(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)
(Reporter)

Comment 6

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

Comment 7

2 years ago
Created attachment 8727598 [details]
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/
Attachment #8727598 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 8

2 years ago
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.
(Assignee)

Comment 9

2 years ago
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 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?
Attachment #8727598 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 11

2 years ago
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!
(Assignee)

Comment 12

2 years ago
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/
Attachment #8727598 - Attachment description: MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r?Gijs → MozReview Request: Bug 1254232 - Use a tree walker to get list of atomic paragraphs to read. r=Gijs

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/26d0246340d3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
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-firefox48: fixed → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.