1139022 - Access keys for EME notification bars are broken because of case difference

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[:Gijs (he/him)]

Some days, I hate camel-casing and the fact that we have a legacy codebase.

This is one of these days, because apparently, notification.xml really wants you to capitalize the K in 'accesskey', and otherwise it just uses "undefined" instead, meaning "U" because that's how clever our widget code is on Windows. :-\

(I blame testing on OS X for not noticing this early enough)

Any reason not to just make notification.xml also check for button.accesskey rather than just the capitalized variant? Dolske?

Comment 1

[:Gijs (he/him)]

Attachment: fix camelcase issue with access key in EME notification bars,

Comment 2

[Marco Mucci [:MarcoM]]

Hi Gijs, can you provide a point value.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8572064 [details] [diff] [review]
fix camelcase issue with access key in EME notification bars,

Review of attachment 8572064 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah this looks fine to me. It's been accessKey in notification.xml since hg@1. I don't see an issue with loosening it (`button.accesskey || button.accessKey`) besides that in the future the callers get pretty loose with what they pass in.

Comment 4

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/6483a9a692b4

Comment 5

[:Gijs (he/him)]

Comment on attachment 8572064 [details] [diff] [review]
fix camelcase issue with access key in EME notification bars,

Approval Request Comment
[Feature/regressing bug #]: bug 1131758
[User impact if declined]: wrong access keys
[Describe test coverage new/current, TreeHerder]: no
[Risks and why]: almost nonexistent, simple case correctness fix on a property name
[String/UUID change made/needed]: no

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6483a9a692b4

Comment 7

[:Gijs (he/him)]

I rolled this into my patches in bug 1136165, so it's fixed on 37 already - just need some aurora uplift love. :-)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b68853d83c4d

Comment 10

[Bogdan Maris, Desktop QA]

Verified as fixed on Windows 7 x64 & Windows 8.1 x64 with latest Nightly 39.0a1 (20150317073344) and latest Aurora 38.0a2 (20150318004012)

Comment 11

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

No additional manual QA needed here for Firefox 37.