1088467 - Space exposed at start of li element when CSS list-style is none

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Str:
1. Open the following URL: data:text/html,<ul><li style="list-style: none;">test</li></ul>
2. Examine the text for the list item accessible.
Expected: "test"
Actual: " test"

It makes sense that a space should be exposed after the prefix (e.g. bullet) if there is one (which wasn't the case before Firefox 32). However, if there is no prefix, a space shouldn't be exposed. This worked as expected in Firefox 32 (no space if no prefix), but regressed in Firefox 33.

This is causing spurious blank lines to appear in NVDA browse mode with screen layout disabled.
Related NVDA issue ticket: http://community.nvda-project.org/ticket/4568

Comment 1

[Marco Zehe (:MarcoZ)]

Most likely a regression from bug 1013160. xidorn, could you take a look, please?

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch

Comment 3

[Marco Zehe (:MarcoZ)]

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

Stealing review from Surkov. I ran with this patch and can confirm the fix. Also ran the a11y test suite and it passes. Thanks for this quick patch!

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

BTW, should we uplift this patch?

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=65a99b37f679

Comment 6

[Marco Zehe (:MarcoZ)]

(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #4)
> BTW, should we uplift this patch?

We can try, at least for Aurora, so the fix gets in the hands of end users sooner. I am not sure what the policy is for beta, since this will have been broken in two consecutive releases (32 and 33). But you can request approval anyway. I've set the Affected flags accordingly.

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Marco Zehe (:MarcoZ) from comment #6)
> (In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #4)
> > BTW, should we uplift this patch?
> 
> We can try, at least for Aurora, so the fix gets in the hands of end users
> sooner. I am not sure what the policy is for beta, since this will have been
> broken in two consecutive releases (32 and 33). But you can request approval
> anyway. I've set the Affected flags accordingly.

Actually, I think it is a regression of bug 966166, not of bug 1013160. Hence it doesn't affect 32.

Comment 8

[James Teh [:Jamie]]

It works as expected in Firefox 32.

Comment 9

[Marco Zehe (:MarcoZ)]

OK, so the flags are correct above. Yeah, request approval for Aurora and beta once this lands on m-c and we'll see where it goes.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c72e16da125b

Comment 11

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

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

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

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

Approval Request Comment
[Feature/regressing bug #]: bug 966166
[User impact if declined]: As described in comment 0
[Describe test coverage new/current, TBPL]: Add one test in mochitest a11y
[Risks and why]: no risk AFAIK
[String/UUID change made/needed]:

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

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

Beta+
Aurora+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b0cb783df31
https://hg.mozilla.org/releases/mozilla-beta/rev/2e54d90546ce