1192901 - Remove remnant accesskey for Disable Addon button

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Théo Chevalier]

Attachment: Screenshot, latest Nightly - French

Bug 1153198 removed the accesskey from the .properties file, but we still have an accesskey in the UI, probably picked randomly by Gecko.

In French "U" has been picked, and it's not a character we have in the word, so we can see "Désactiver [NAME] (U)" in the UI (see attached screenshot). I think we have the same issue in English.

We should get rid of this accesskey we don't control :)

Comment 1

[YF (Yang)]

Attachment: bug_1192901.patch

This is a workaround for bug 1153198.

I see the accesskey = undefined in DOM, looks the appendNotification not recognize the null or undefined for accesskey (http://hg.mozilla.org/mozilla-central/annotate/7a19194812eb/toolkit/content/widgets/notification.xml#l125).

This will leave an empty property in the element, but looks it works well.

Comment 2

[YF (Yang)]

Attachment: testcase using Scratchpad (chrome)

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 8646017 [details] [diff] [review]
bug_1192901.patch

I'm turning this review over to Gijs, because David is on vacation this week

Comment 4

[:Gijs (he/him)]

Comment on attachment 8646017 [details] [diff] [review]
bug_1192901.patch

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

I'd really prefer to fix appendNotification to not be dumb, so we won't make this mistake again. But it looks like we might want to uplift this and the other thing (maybe without touching the locale files), so let's take this for now and I'll file a followup. If you feel like you could, it'd be great if you could put up a patch for that as well!

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/66d1159b02a6

Comment 6

[YF (Yang)]

(In reply to Pulsebot from comment #5)
> https://hg.mozilla.org/integration/fx-team/rev/66d1159b02a6

Why is the patch be automatically checked out, even the not have "checkin-needed" keyword? For now, we just need to checkin and uplift the patch of bug 1194503. Take any action?

Comment 7

[:Gijs (he/him)]

(In reply to YF (Yang) from comment #6)
> (In reply to Pulsebot from comment #5)
> > https://hg.mozilla.org/integration/fx-team/rev/66d1159b02a6
> 
> Why is the patch be automatically checked out, even the not have
> "checkin-needed" keyword? For now, we just need to checkin and uplift the
> patch of bug 1194503. Take any action?

Not so much automatically, I just landed both patches for you. :-)

Yes, we should uplift this. You could request uplift by setting the "approval-mozilla-aurora" and "approval-mozilla-beta" flags on the patch attachment ( https://bugzilla.mozilla.org/attachment.cgi?id=8646017&action=edit ), and filling out the questions that doing that will put in the comment box. Let me know if you want me to do it or need help answering them.

Comment 8

[YF (Yang)]

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to YF (Yang) from comment #6)
> > (In reply to Pulsebot from comment #5)
> > > https://hg.mozilla.org/integration/fx-team/rev/66d1159b02a6
> > 
> > Why is the patch be automatically checked out, even the not have
> > "checkin-needed" keyword? For now, we just need to checkin and uplift the
> > patch of bug 1194503. Take any action?
> 
> Not so much automatically, I just landed both patches for you. :-)

Yes, thank you. But the checkin by Pulsebot, a "bot", I first see it and the bug not have a keyword request, I'm confused for the process.

> Yes, we should uplift this. You could request uplift by setting the
> "approval-mozilla-aurora" and "approval-mozilla-beta" flags on the patch
> attachment (
> https://bugzilla.mozilla.org/attachment.cgi?id=8646017&action=edit ), and
> filling out the questions that doing that will put in the comment box. Let
> me know if you want me to do it or need help answering them.

Yes, I already know that.

I intend to give up this patch of this bug, I need to take any mark or action?

And, I planning to uplift the attachment 8647893 [details] [diff] [review] when it checkin to m-c, the seems to be a requirement for uplift.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/66d1159b02a6

Comment 10

[YF (Yang)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> https://hg.mozilla.org/mozilla-central/rev/66d1159b02a6

Do we need a backed out (cleanup) for this changeset?

Comment 11

[:Gijs (he/him)]

Let's wait and see if we can get the other one uplifted.