306433 - Strange cloned accesskeys in Pref window <description> tags

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Marek Stępień [:marcoos, inactive]]

All of the tabs except History and Cache in the Privacy tab of the pref window
somehow copy accesskeys from the widgets inside the tab to the <description>
tag, for example the Cookies tab <description> copies its accesskey from the
'allow sites to set cookies' checkbox.

This causes some nasty problems for l10n teams. For example, the accesskey for
the "allow pages to set cookies" checkbox is in the Polish l10n "l", but the
letter 'l' is not used in the "Cookies are blahblah" description in that tab, so
a "(L)" is added to the description and looks really bad. I'll attach a
screenshot showing this problem in a moment.

These copied accesskeys should be removed from the non-clickable widgets.

Comment 1

[Marek Stępień [:marcoos, inactive]]

Attachment: Screenshot showing the problem in the "pl" l10n

This screenshots shows the cloned accesskeys in the Cookies, Download History
and Passwords tabs of the privacy preferences.

Comment 2

[Aaron Leventhal]

We need to not underline accesskeys on description elements.

Comment 3

[Marek Stępień [:marcoos, inactive]]

Why should they have accesskeys at all? (And if they should have ones for some
strane reasons, it really shouldn't be copies of other elements' accesskeys)

Comment 4

[Aaron Leventhal]

(In reply to comment #3)
> Why should they have accesskeys at all? (And if they should have ones for some
> strane reasons, it really shouldn't be copies of other elements' accesskeys)

The accesskey must be on the control.

In the <label control="[id]"> case we check the control for the accesskey
because it is sometimes specified there.

We recently added the control attribute to descriptions in the pref window so
that screen readers would read the appropriate description as a user tabbed
around to different controls.

I can look at this bug -- it's probably a simple fix in text.xml.

Comment 5

[Aaron Leventhal]

I'm seeing this on base en-US builds as well, not sure why. The <description>
binding uses text-base which doesn't underline accesskey.

Comment 6

[[e-mail]]

Sorry, I can't use Firefox as a test case.

Comment 7

[Aaron Leventhal]

Working on a fix.

Comment 8

[Aaron Leventhal]

Attachment: Use regular text-base for description, even if it has control attribute, because we don't want to underline accesskeys in it.

Comment 9

[Aaron Leventhal]

This does make it so clicking on a description doesn't focus the control it
points to, but that doesn't need to happen anyway. That's the function of a
<label> not a <description>. The description is static text in a dialog, which
gives supplemental information. We have a control attribute so that screen
readers can read the description for each control when it exists.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 194372 [details] [diff] [review]
Use regular text-base for description, even if it has control attribute, because we don't want to underline accesskeys in it.

This sounds fine to me as described, but I'm not sure if there is any code out
there that depends on the previous <description> behaviour... I'd guess not
many (if any), though.

Comment 11

[Aaron Leventhal]

The only difference between the 2 bindings is the click to focus and accesskey
underlining. In the case of the click to focus you can still click on the
control. Based on that, any potential regression is a tiny issue.

Comment 12

[Chris Beard]

Comment on attachment 194372 [details] [diff] [review]
Use regular text-base for description, even if it has control attribute, because we don't want to underline accesskeys in it.

-'d for approval1.8b4.	let's get this baking on the trunk for a few more days
to ensure that it's fixed and doesn't cause additional regressions.  please
re-request approval.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 194372 [details] [diff] [review]
Use regular text-base for description, even if it has control attribute, because we don't want to underline accesskeys in it.

re-requesting approval. we need to get this verified on the trunk first though.

Comment 14

[Tracy Walker [:tracy]]

on lates PL build I saw the (L) on first entry to The Privacy option...but upon
return it was gone.  

I don't see it on en-ES

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 194372 [details] [diff] [review]
Use regular text-base for description, even if it has control attribute, because we don't want to underline accesskeys in it.

Sounds like this is working. a=asa for branch checkin. time is short so please
land this quickly. thanks.