Remove remnant accesskey for Disable Addon button

RESOLVED FIXED in Firefox 43

Status

()

Toolkit
Performance Monitoring
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tchevalier, Assigned: YF (Yang))

Tracking

unspecified
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Created attachment 8645782 [details]
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 :)
(Assignee)

Updated

2 years ago
Assignee: nobody → yfdyh000
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8646017 [details] [diff] [review]
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.
Attachment #8646017 - Flags: review?(dteller)
(Assignee)

Comment 2

2 years ago
Created attachment 8646018 [details]
testcase using Scratchpad (chrome)
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
Attachment #8646017 - Flags: review?(dteller) → review?(gijskruitbosch+bugs)

Comment 4

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

Comment 5

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/66d1159b02a6
(Assignee)

Comment 6

2 years ago
(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?
Depends on: 1194503

Comment 7

2 years ago
(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.
Flags: needinfo?(yfdyh000)
(Assignee)

Comment 8

2 years ago
(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.
Flags: needinfo?(yfdyh000)
https://hg.mozilla.org/mozilla-central/rev/66d1159b02a6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 10

2 years ago
(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?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 11

2 years ago
Let's wait and see if we can get the other one uplifted.
Flags: needinfo?(gijskruitbosch+bugs)
You need to log in before you can comment on or make changes to this bug.